Skip to content
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

Merged
merged 1 commit into from
Oct 10, 2019

Conversation

dusan19
Copy link
Contributor

@dusan19 dusan19 commented Oct 8, 2019

…opic

Bugfix when getting all available messages on the subsystem_info topic in commander

@@ -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()) ) {
Copy link
Contributor

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?

Copy link
Contributor

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;
		}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, changed

Copy link
Contributor Author

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

@dusan19 dusan19 force-pushed the commander_subsystem_get_bugfix branch from 3e97bb4 to 855a884 Compare October 8, 2019 11:57
julianoes
julianoes previously approved these changes Oct 8, 2019
Copy link
Contributor

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

Thx.

julianoes
julianoes previously approved these changes Oct 9, 2019
Copy link
Contributor

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

Thx!

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