chore(hardware): Add some logging around what I think is causing some can errors #17117
+6
−0
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
sfoster1
requested changes
Dec 16, 2024
| @@ -469,6 +469,10 @@ def _remove_move_group( | |||
| f"Received completion for {node_id} group {group_id} seq {seq_id}" | |||
| f", which {'is' if in_group else 'isn''t'} in group" | |||
| ) | |||
| if not self._moves[group_id] and len(self._moves[group_id]) == 0: | |||
There was a problem hiding this comment.
shouldn't this be self._moves[group_id] and len(self._moves[group_id]) == 0? this check: not self._moves[group_id] and len(self._moves[group_id]) == 0 is true if both are true, but aren't we asserting that a bug causes len(self._moves[group_id] == 0 to be true and not self._moves[group_id] to be false at the same time?
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.
Overview
We occasionally get some ABR issues where the can bus says it's not receiving ACKs or that a move group says it didn't get all expected nodes, but reports [] as the missing nodes.
I think the second bug is because we're checking with
if not self._moves[group_id]instead of checkingif len(self._moves[group_id]) == 0:Which should be equivalent but I think there may be some python cleanup bug that reports the list as true due to a remnant that hasn't been garbage collected yet.Test Plan and Hands on Testing
Changelog
Review requests
Risk assessment