-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
Commander: bugfix: get all available messages on the subsystem_info t… #13121
Conversation
src/modules/commander/Commander.cpp
Outdated
@@ -1737,7 +1737,7 @@ Commander::run() | |||
|
|||
/* update subsystem info which arrives from outside of commander*/ | |||
do { | |||
if (subsys_sub.updated()) { | |||
if ( (updated=subsys_sub.updated()) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, this updated
bool is defined 500 lines above. It looks like it's not used anywhere else. Could you move it down to here where it's used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, can't you just simplify it like this?
/* update subsystem info which arrives from outside of commander*/
while (subsys_sub.updated()) {
subsystem_info_s info{};
subsys_sub.copy(&info);
set_health_flags(info.subsystem_type, info.present, info.enabled, info.ok, status);
status_changed = true;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also tested, it gets at most 5 messages in 1 commander loop, which is limited by the messages orb queue size
3e97bb4
to
855a884
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx.
855a884
to
2c84eff
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx!
2c84eff
to
1a2d756
Compare
…opic
Bugfix when getting all available messages on the subsystem_info topic in commander