Skip to content

Add add_analyzer functionality#3

Open
MartinCornelis2 wants to merge 13 commits into
ros2from
feature/add-analyzers-through-parameters
Open

Add add_analyzer functionality#3
MartinCornelis2 wants to merge 13 commits into
ros2from
feature/add-analyzers-through-parameters

Conversation

@MartinCornelis2
Copy link
Copy Markdown
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>

Comment thread diagnostic_aggregator/src/aggregator.cpp
Comment thread diagnostic_aggregator/src/aggregator.cpp
Comment thread diagnostic_aggregator/src/add_analyzer.cpp Outdated

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

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
Copy Markdown
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
Copy Markdown

@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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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