-
Notifications
You must be signed in to change notification settings - Fork 28
Add firmware errors to diagnostics #234
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
base: rc/jazzy/2.7
Are you sure you want to change the base?
Add firmware errors to diagnostics #234
Conversation
The CI won't succeed until clearpathrobotics/clearpath_msgs/pull/81 is released/used by CI. Otherwise I think you're good to merge. |
This diagnostic should be enabled / disabled based on the platform type to prevent nuisance errors on non-A300 platforms. The parameters that are generated and passed to the diagnostics node are handled here: clearpath_common/clearpath_generator_common/clearpath_generator_common/param/platform.py Line 369 in 305f5bb
|
@hilary-luo I believe the plan is to create a new message so this may not be relevant anymore. But in this version the new diagnostics wouldn't show an error if no error codes were present in the status message. It would only show an error if the status message wasn't published. Does this not satisfy your concern? |
Perhaps error was not the appropriate wording. There has been intentional effort to not show extra diagnostic entries / values when they are known to be irrelevant for the platform. At best they clutter the diagnostics and at worst could potentially be misleading (ie. a diagnostic that says there are no firmware errors when that may not be true because there is no reporting on that platform). I am of the opinion that this falls into that bucket that it should be hidden for those where we do not and cannot actually report on firmware errors. |
Thanks for the clarification. Good notes for whoever picks this up again. |
No description provided.