python: fix issues 198 & 203 #204
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some modems will report partial cellular connection details, especially during the startup phase, at which point the modem considers the connection to be established, but does not yet report what techniology is used. Fixes: Orange-OpenSource#198 Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
Commit c482cb1 (python/vehicle: use iot3.core.mqtt) introduced a regression in the propagation of messages from the remote broker to the local one. The API for the IoT3 Core SDK extensively uses keywords rather than positional parameters, and although this was properly accounted when publishing messages on the remote broker, this was overlooked when publishing messages on the local broker. Such an issue would have raised an exception, and we would have caught it quite easily. Alas, in an attempt at being robust when decoding the received messages, we decided for a blanket coverage of all exceptions, no matter what. This indeed hid the TypeError exception that is raised when improperly calling the SDK... So, now, we could just catch KeyError when looking up the source_uuid field, but that would assume the message was decoded as a dict, which we have no guarantee it was. If that were not the case, i.e.e the message is not an ITS message, and contains any other JSON object that is not a dictionary, that would _also_ raise TypeError, so we would have to catch it anyway to ignore it. And we would not have been able to notice the TypeError raised by improperly calling the SDK either anyway... Sigh... For now, just fix calling the SDK; we'll handle the rest later (Famous Last Words™)... Fixes: Orange-OpenSource#203 Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
tigroo
approved these changes
Nov 8, 2024
| @@ -193,7 +193,7 @@ def msg_cb( | |||
| payload_d.get("source_uuid", None) != self.cfg["instance-id"] | |||
| or self.cfg["mirror-self"] | |||
| ): | |||
| self.mqtt_mirror.publish(topic, payload) | |||
| self.mqtt_mirror.publish(topic=topic, payload=payload) | |||
There was a problem hiding this comment.
Yes, it does. As the commit log mentions, the API expects keyword-only arguments, not positional arguments.
And this fix is the one that fixes propagating messages to the local broker (issue #203).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes:
How to test:
Expected results: