René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"`NmtBase.state` should always return a `str`","articleBody":"## Problem\r\n\r\n`NmtBase.state` is documented and annotated as always returning a string, however there is a code path that may return an `int`:\r\n\r\nhttps://github.com/christiansandberg/canopen/blob/62e9c1f851cb8721355d22c8571f2b41599c1b36/canopen/nmt.py#L74-L92\r\n\r\nThe code path in question is highly unlikely; the `NmtBase._state` member is set in the following places:\r\n\r\n- ✅ `NmtBase.__init__`: explicitly set to 0\r\n- ✅ `NmtBase.on_command`: set _iff_ the new state is a valid/known state\r\n- ✅ `NmtBase.send_command`: set _iff_ the new state is a valid/known state\r\n- ❓ `NmtMaster.on_heartbeat`: set to whatever was decoded from the heartbeat message, which _should_ be fine for any compliant device\r\n\r\n## Possible solutions\r\n\r\nIn `NmtMaster.on_heartbeat`, only set `_state` if it is a valid state; if it's not, `log.error(...)` or raise an exception.\r\n\r\nIn `NmtBase.state`, either raise an exception if `._state` is not a valid state, or return something a la `f\"UNKNOWN STATE {self._state}\"`.\r\n\r\n\u003cdetails\u003e\r\n\u003csummary\u003eDraft diff\u003c/summary\u003e\r\n\r\n```diff\r\ndiff --git a/canopen/nmt.py b/canopen/nmt.py\r\nindex 401ad15..1316a5c 100644\r\n--- a/canopen/nmt.py\r\n+++ b/canopen/nmt.py\r\n@@ -86,10 +86,8 @@ class NmtBase:\r\n         - 'RESET'\r\n         - 'RESET COMMUNICATION'\r\n         \"\"\"\r\n-        if self._state in NMT_STATES:\r\n-            return NMT_STATES[self._state]\r\n-        else:\r\n-            return self._state\r\n+        assert self._state in NMT_STATES:\r\n+        return NMT_STATES[self._state]\r\n \r\n     @state.setter\r\n     def state(self, new_state: str):\r\n@@ -122,6 +120,12 @@ class NmtMaster(NmtBase):\r\n             logger.debug(\"Received heartbeat can-id %d, state is %d\", can_id, new_state)\r\n             for callback in self._callbacks:\r\n                 callback(new_state)\r\n+            if new_state not in NMT_STATES:\r\n+                log.error(\r\n+                    \"Received heartbeat can-id %d with invalid state %d\",\r\n+                    can_id, new_state\r\n+                )\r\n+                return\r\n             if new_state == 0:\r\n                 # Boot-up, will go to PRE-OPERATIONAL automatically\r\n                 self._state = 127\r\n```\r\n\r\n\u003c/details\u003e","author":{"url":"https://github.com/erlend-aasland","@type":"Person","name":"erlend-aasland"},"datePublished":"2024-07-08T09:29:45.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":11},"url":"https://github.com/500/canopen/issues/500"}