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

ROS2 port of feature to report stale when no errors but stale items #9

Merged

Conversation

MCFurry
Copy link
Member

@MCFurry MCFurry commented Apr 29, 2024

@MCFurry MCFurry requested review from Rayman and Timple April 29, 2024 11:53
@MCFurry MCFurry changed the title ROS2 port of feature to report stale when no errors but stale items [WIP] ROS2 port of feature to report stale when no errors but stale items Apr 29, 2024
@MCFurry MCFurry changed the title [WIP] ROS2 port of feature to report stale when no errors but stale items ROS2 port of feature to report stale when no errors but stale items Apr 29, 2024
@MCFurry
Copy link
Member Author

MCFurry commented Apr 29, 2024

Not sure why rolling is failing, compiles fine locally in a rolling docker...

@Timple
Copy link
Member

Timple commented Apr 30, 2024

Isn't this the same as: ros#315
Which is already merged and released upstream?

@MCFurry
Copy link
Member Author

MCFurry commented Apr 30, 2024

Not sure why rolling is failing, compiles fine locally in a rolling docker...

Isn't this the same as: ros#315 Which is already merged and released upstream?

Nope, that PR specifically targets discard analyzers. This PR makes sure that a stale diagnostic makes the toplevel output stale instead of error.

@Timple
Copy link
Member

Timple commented Apr 30, 2024

Not sure why rolling is failing, compiles fine locally in a rolling docker...

Can rebase the nobleo-ros2 branch to the ros2 branch such that we have a clean start again? Some of our PR's did get merged like the cpu monitor and stuff.

That might fix the CI as well would be my guess.

@MCFurry
Copy link
Member Author

MCFurry commented Apr 30, 2024

Not sure why rolling is failing, compiles fine locally in a rolling docker...

Can rebase the nobleo-ros2 branch to the ros2 branch such that we have a clean start again? Some of our PR's did get merged like the cpu monitor and stuff.

That might fix the CI as well would be my guess.

That would be a good idea indeed!

@MCFurry
Copy link
Member Author

MCFurry commented Apr 30, 2024

Syncing done, let's see what the CI says now

@@ -256,6 +259,7 @@ void Aggregator::publishData()
diag_toplevel_state.values = msg_to_report->values;
}

non_ok_status_depth = 0;
Copy link

Choose a reason for hiding this comment

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

What is this new variable?

@@ -221,7 +221,7 @@ void Aggregator::publishData()
diag_toplevel_state.name = "toplevel_state";
diag_toplevel_state.level = DiagnosticStatus::STALE;
int max_level = -1;
int min_level = 255;
int8_t max_level_without_stale = 0;
int non_ok_status_depth = 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

@Rayman it is here already and simply reset on line 262.

@MCFurry MCFurry merged commit c68ec38 into nobleo-ros2 May 1, 2024
18 checks passed
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