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

Add add_analyzer functionality #3

Open
wants to merge 13 commits into
base: ros2
Choose a base branch
from

Conversation

MartinCornelis2
Copy link
Member

Applied the following changes:

  • Move the creation of analyzer_group_ and other_analyzer_ from the constructor to a new function initAnalyzers().

  • Added a subscriber to parameter events param_sub_ that triggers a callback parameterCallback.

  • Check in the callback if this node got any new paramters -> if true, lock the mutex and call initAnalyzers() again.

  • Created an add_analyzer node that forwards its own parameters to the diagnostic_aggregator by sending a request to /diagnostics_agg/set_parameters_atomically service

  • The add_analyzer looks for all parameters starting with the prefix analyzers., converts them to a parameter_msg and sends them together as one SetParametersAtomically::Request

Example usage:

<node name="add_steering_ecu_analyzer" pkg="diagnostic_aggregator" exec="add_analyzer" >
        <param from="$(find-pkg-share harvey)/param/diagnostic_analyzers/steering_ecu_analyzer.yaml" allow_substs="true"/>
        <remap from="/diagnostics_agg/set_parameters_atomically" to="/diagnostics_autonomy/set_parameters_atomically" />
</node>

diagnostic_aggregator/src/aggregator.cpp Show resolved Hide resolved
diagnostic_aggregator/src/add_analyzer.cpp Outdated Show resolved Hide resolved

private:
rclcpp::Client<rcl_interfaces::srv::SetParametersAtomically>::SharedPtr client_;
std::string prefix_ = "analyzers.";

Choose a reason for hiding this comment

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

perhaps a little more clarity could help with the name. Something like a node_param_ns_,?

@Timple
Copy link
Member

Timple commented Jan 12, 2024

Great, please fix the linting (See CI) and rebase of the ros2 branch. Otherwise we cannot forward this PR as it would also contain ros#324

@Timple Timple force-pushed the feature/add-analyzers-through-parameters branch from 7ca53cd to 8c85be7 Compare January 12, 2024 08:17
@Timple Timple changed the base branch from feature/critical-on-every-degradation to ros2 January 12, 2024 08:18
@Timple Timple force-pushed the feature/add-analyzers-through-parameters branch from 8c85be7 to 863d110 Compare January 12, 2024 08:18
@Timple Timple force-pushed the feature/add-analyzers-through-parameters branch from 817e6f6 to f3b4b35 Compare January 12, 2024 15:10
@Timple Timple force-pushed the feature/add-analyzers-through-parameters branch from f3b4b35 to d8c28eb Compare January 12, 2024 15:13
/*********************************************************************
* Software License Agreement (BSD License)
*
* Copyright (c) 2009, Willow Garage, Inc.
Copy link

@Rayman Rayman Mar 1, 2024

Choose a reason for hiding this comment

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

Did you write this code or adapted this from somewhere? If you wrote it yourself, this should be the BV where you work for, so Copyright 2024 Nobleo Autonomous Solutions B.V.

Copy link
Member Author

Choose a reason for hiding this comment

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

Wrote it myself but just copied the pasta for the notice and forgot to make it Nobleo Technology or whatever it should be, will update.

* copyright notice, this list of conditions and the following
* disclaimer in the documentation and/or other materials provided
* with the distribution.
* * Neither the name of the Willow Garage nor the names of its
Copy link

Choose a reason for hiding this comment

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

Just use the standard BSD sentences:

3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should I change it? Now the license matches all the other files in the repo.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, yeh I see what you mean now it can't stay willow garage :P

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.

4 participants