Skip to content

Commit 9098fb9

Browse files
author
sidey79
committed
feat(mqtt): make req_id optional for all MQTT command payloads
The 'req_id' field in MQTT command payloads is now optional. This required changes to the JSON schema validation and the response handling logic. - Updates `BASE_SCHEMA` and `SEND_MSG_SCHEMA` in `signalduino/commands.py` to remove the `req_id` requirement. - Modifies `MqttCommandDispatcher.dispatch` to return `None` for `req_id` if not present in the payload. - Adjusts `MqttPublisher._handle_command` to correctly extract the optional `req_id` for success and error responses. - Adds `test_controller_handles_get_frequency_without_req_id` to verify the new optional behavior. - Updates `docs/architecture/proposals/mqtt_set_commands.adoc` to document the optional nature of `req_id`.
1 parent c3c36c4 commit 9098fb9

5 files changed

Lines changed: 89 additions & 19 deletions

File tree

.vscode/settings.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252
"host": "mqtt",
5353
"port": 1883,
5454
"clientId": "vsmqtt_client_db93",
55-
"savedSubscriptions": ['signalduino/v1/responses','signalduino/v1/commands','signalduino/v1/errors']
55+
"savedSubscriptions": ['signalduino/v1/responses','signalduino/v1/messages','signalduino/v1/errors']
5656
}
5757
]
5858
}

docs/architecture/proposals/mqtt_set_commands.adoc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,13 @@ Die Methoden in `SignalduinoCommands` sind für die Umrechnung und das Senden de
9393
* **Befehlssatz:** Verwendet den generischen Register-Write-Befehl `W1D`. Die Umrechnungslogik von dB-Wert in den Registerindex (`00`-`07`) erfolgt in der Python-Implementierung.
9494
* **Serial Command:** `W1D<Index>`, gefolgt von `cc1101_write_init()`.
9595

96+
=== 3.2. MQTT Payload und `req_id`-Behandlung
97+
98+
Das Feld `req_id` in allen MQTT-Command-Payloads (sowohl `set/...` als auch `get/...` und `command/...`) ist ab sofort *optional*.
99+
100+
* **Anfrage:** Wird `req_id` nicht in der Anfrage gesendet, darf die Payload-Validierung nicht fehlschlagen.
101+
* **Antwort:** Bei Fehlen der `req_id` in der Anfrage wird das Feld `req_id` in der Antwort-Payload (Topics `responses` und `errors`) den Wert `null` (JSON null) annehmen.
102+
96103
== 4. Fehlerbehandlung und Validierung
97104
1. **Validierung (Dispatcher):** Die MQTT-Payloads werden durch das in [`SignalduinoCommands.COMMAND_MAP`](signalduino/commands.py) definierte `schema` (z.B. `FREQ_SCHEMA`, `DATARATE_SCHEMA`) vor dem Aufruf der Controller-Methode validiert.
98105
2. **Serial-Kommunikation:** `SignalduinoCommands._send_command` wird bei Timeout eine `SignalduinoCommandTimeout` werfen. Diese wird im `SignalduinoController` gefangen und als NACK-Statusmeldung zurückgegeben.

signalduino/commands.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ async def cc1101_write_init(self) -> None:
416416
"value": {"type": ["string", "number", "boolean", "null"], "description": "Main value for SET commands."},
417417
"parameters": {"type": "object", "description": "Additional parameters for complex commands (e.g., sendMsg)."},
418418
},
419-
"required": ["req_id"],
419+
"required": [], # req_id ist jetzt optional
420420
"additionalProperties": False
421421
}
422422

@@ -425,8 +425,8 @@ def create_value_schema(value_schema: Dict[str, Any]) -> Dict[str, Any]:
425425
schema = BASE_SCHEMA.copy()
426426
schema['properties'] = BASE_SCHEMA['properties'].copy()
427427
schema['properties']['value'] = value_schema
428-
# Erstelle eine neue 'required'-Liste, um das Problem der flachen Kopie und der Mutationen zu beheben
429-
schema['required'] = BASE_SCHEMA['required'] + ['value']
428+
# Da BASE_SCHEMA['required'] jetzt leer ist, fügen wir nur 'value' hinzu
429+
schema['required'] = ['value']
430430
return schema
431431

432432
# --- CC1101 SPEZIFISCHE SCHEMATA (PHASE 2) ---
@@ -492,7 +492,7 @@ def create_value_schema(value_schema: Dict[str, Any]) -> Dict[str, Any]:
492492
"additionalProperties": False,
493493
}
494494
},
495-
"required": ["req_id", "parameters"],
495+
"required": ["parameters"],
496496
"additionalProperties": False
497497
}
498498

@@ -598,6 +598,6 @@ async def dispatch(self, command_path: str, payload: str) -> Dict[str, Any]:
598598
# 4. Prepare Response
599599
return {
600600
"status": "OK",
601-
"req_id": payload_dict["req_id"],
601+
"req_id": payload_dict.get("req_id", None), # req_id ist jetzt optional
602602
"data": result
603603
}

signalduino/mqtt.py

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -162,17 +162,29 @@ async def _handle_command(self, command_name: str, payload: str) -> None:
162162

163163
self.logger.info("Handling command: %s with payload: %s", command_name, payload)
164164

165-
req_id = "NO_REQ_ID" # Standard-req_id, falls das Parsing fehlschlägt
165+
req_id: Optional[str] = None
166+
167+
# Versuche, req_id aus dem Payload zu extrahieren, falls es sich um gültiges JSON handelt.
168+
try:
169+
payload_dict = json.loads(payload)
170+
req_id = payload_dict.get("req_id")
171+
except json.JSONDecodeError:
172+
# Der Payload ist kein gültiges JSON. req_id bleibt None, und der Dispatcher
173+
# wird dies als CommandValidationError behandeln, wenn er json.loads erneut aufruft.
174+
pass
166175

167176
try:
168177
# Der Dispatcher gibt ein Ergebnis-Dictionary mit 'status', 'req_id', 'data' zurück.
169178
result = await self.dispatcher.dispatch(command_name, payload)
170-
req_id = result.get("req_id", req_id)
179+
180+
# Der Dispatcher kann req_id als None zurückgeben, wenn sie nicht im Payload war.
181+
# Wir überschreiben req_id mit dem Ergebnis, um Konsistenz zu gewährleisten.
182+
req_id = result.get("req_id") # Kann None sein
171183

172184
response_payload = {
173185
"command": command_name,
174186
"success": True,
175-
"req_id": req_id,
187+
"req_id": req_id, # Kann None sein, was in JSON zu null wird
176188
"payload": result.get("data"),
177189
}
178190

@@ -185,33 +197,27 @@ async def _handle_command(self, command_name: str, payload: str) -> None:
185197

186198
except (CommandValidationError, SignalduinoCommandTimeout) as e:
187199
self.logger.warning("Command failed (Validation/Timeout): %s: %s", command_name, e)
188-
# req_id kann von der Validierung extrahiert werden, falls der Payload gültiges JSON ist
189-
try:
190-
# Da der Dispatcher JSON.loads(payload) aufruft und fehlschlagen kann, wenn JSON ungültig ist,
191-
# müssen wir hier vorsichtiger sein. Ist der Payload gültig, holen wir die req_id.
192-
payload_dict = json.loads(payload)
193-
req_id = payload_dict.get("req_id", req_id)
194-
except json.JSONDecodeError:
195-
pass # req_id bleibt "NO_REQ_ID"
196200

197201
await self.publish_simple(
198202
subtopic="errors",
199203
payload=json.dumps({
200204
"command": command_name,
201205
"success": False,
202-
"req_id": req_id,
206+
"req_id": req_id, # Verwendet die oben extrahierte (oder None)
203207
"error": str(e),
204208
}),
205209
retain=False
206210
)
207211
except Exception:
212+
# Wenn ein interner Fehler auftritt (z.B. im Controller),
213+
# verwenden wir die zuvor extrahierte req_id.
208214
self.logger.exception("Internal error during command dispatching: %s", command_name)
209215
await self.publish_simple(
210216
subtopic="errors",
211217
payload=json.dumps({
212218
"command": command_name,
213219
"success": False,
214-
"req_id": req_id,
220+
"req_id": req_id, # Verwendet die oben extrahierte (oder None)
215221
"error": "Internal server error during command execution.",
216222
}),
217223
retain=False

tests/test_mqtt_commands.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,63 @@ async def test_controller_handles_get_frequency(signalduino_controller, mock_aio
299299
call(command='C0F', expect_response=True, timeout=2.0, response_pattern=expected_pattern),
300300
])
301301

302+
@pytest.mark.asyncio
303+
async def test_controller_handles_get_frequency_without_req_id(signalduino_controller, mock_aiomqtt_client_cls, mock_logger):
304+
"""
305+
Testet den 'get/cc1101/frequency' MQTT-Befehl, wenn keine req_id gesendet wird.
306+
Die resultierende Response sollte eine req_id von None enthalten (was in JSON zu null wird).
307+
"""
308+
# Wir benötigen 'call' aus unittest.mock, das am Anfang der Datei importiert wurde.
309+
310+
# Simuliere die Antworten für die drei Register-Lesebefehle (C0D, C0E, C0F)
311+
# FREQ2 (0D) -> 0x21
312+
# FREQ1 (0E) -> 0x62
313+
# FREQ0 (0F) -> 0x00
314+
mock_responses = [
315+
"C0D = 21", # FREQ2
316+
"C0E = 62", # FREQ1
317+
"C0F = 00", # FREQ0
318+
]
319+
320+
send_command_mock = AsyncMock(side_effect=mock_responses)
321+
322+
# Überschreibe die interne Referenz im Commands-Objekt, da es sich um ein gebundenes Callable handelt
323+
signalduino_controller.commands._send_command = send_command_mock
324+
325+
# 1. Dispatcher und Payload vorbereiten (keine req_id!)
326+
command_path = "get/cc1101/frequency"
327+
mqtt_payload = '{}'
328+
329+
dispatcher = MqttCommandDispatcher(controller=signalduino_controller)
330+
331+
# 2. Asynchronen Kontext des Controllers starten
332+
async with signalduino_controller:
333+
334+
# 3. Dispatch ausführen
335+
result = await dispatcher.dispatch(command_path, mqtt_payload)
336+
337+
# 4. Assertions
338+
339+
# Berechne erwartete Frequenz
340+
FXOSC = 26.0
341+
DIVIDER = 65536.0
342+
f_reg = (0x21 << 16) | (0x62 << 8) | 0x00
343+
expected_frequency = (FXOSC / DIVIDER) * f_reg
344+
expected_frequency_rounded = round(expected_frequency, 4)
345+
346+
assert result['status'] == "OK"
347+
assert result['req_id'] is None # <- CRITICAL: Überprüfe, dass req_id None ist
348+
assert result['data']['frequency_mhz'] == expected_frequency_rounded
349+
350+
# Überprüfe, ob send_command mit den korrekten Argumenten aufgerufen wurde (gleiche Calls wie zuvor)
351+
expected_pattern = re.compile(r'C[A-Fa-f0-9]{2}\s=\s[0-9A-Fa-f]+$|ccreg 00:')
352+
353+
send_command_mock.assert_has_calls([
354+
call(command='C0D', expect_response=True, timeout=2.0, response_pattern=expected_pattern),
355+
call(command='C0E', expect_response=True, timeout=2.0, response_pattern=expected_pattern),
356+
call(command='C0F', expect_response=True, timeout=2.0, response_pattern=expected_pattern),
357+
])
358+
302359
@pytest.mark.asyncio
303360
async def test_controller_handles_set_factory_reset(signalduino_controller, mock_aiomqtt_client_cls, mock_logger):
304361
"""Test handling of the 'set/factory_reset' command, ensuring the 'e' command is sent."""

0 commit comments

Comments
 (0)