Skip to content

Add error array to Status.msg #81

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: rc/jazzy/2.7
Choose a base branch
from

Conversation

mikehosmar
Copy link
Member

Allows reporting firmware errors along with status message

@mikehosmar mikehosmar requested a review from a team as a code owner July 8, 2025 19:51
@mikehosmar mikehosmar requested review from tonybaltovski and roni-kreinin and removed request for a team July 8, 2025 19:51
Copy link
Contributor

@roni-kreinin roni-kreinin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this message be sent out every time the error array changes? If so that would break the 1hz frequency that we expect status to publish at. If not, we could potentially miss errors that go active and get cleared in between the 1 hz publishes.

@mikehosmar
Copy link
Member Author

The publish frequency hasn't changed. Response time wasn't a priority.

@tonybaltovski
Copy link
Member

Can we published on an error outside of the published frequency?

@roni-kreinin
Copy link
Contributor

We can, its just going to cause warnings/errors in diagnostics if it publishes too fast.

My concern is not with response time but rather that we can just miss errors entirely if they clear within a second.

@mikehosmar
Copy link
Member Author

If the error lasts less than a second is there anything to be done?

@roni-kreinin
Copy link
Contributor

It should be reported regardless. Like if we are seeing an intermittent error because a cable is loose and occasionally losing contact, the user would want to know that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants