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

Logging macro conditional on a latch/flipflop #456

Open
russkel opened this issue Mar 19, 2024 · 5 comments
Open

Logging macro conditional on a latch/flipflop #456

russkel opened this issue Mar 19, 2024 · 5 comments
Labels
help wanted Extra attention is needed

Comments

@russkel
Copy link

russkel commented Mar 19, 2024

A common pattern of logging I have come across in hardware integration is a flipflop kind of pattern:

e.g. Hardware callback reports at 10Hz "BAD" for X seconds, and then reports "GOOD" when hardware state good:

void hardware_callback(const std::string state) {
  if (state == "BAD") {
  RCLCPP_INFO(get_logger(), "hardware not ready yet");
 }
  if (state == "GOOD") {
  RCLCPP_INFO(get_logger(), "System in good state, ready.");
 }
}

Would spam debug msgs:

hardware not ready yet
hardware not ready yet
hardware not ready yet
hardware not ready yet
// and finally system swaps over
System in good state, ready.
System in good state, ready.
System in good state, ready.
System in good state, ready.
System in good state, ready.
// and then back again
hardware not ready yet
hardware not ready yet
hardware not ready yet
hardware not ready yet
hardware not ready yet

Ideally it would be something like:

void hardware_callback(const std::string state) {
  if (state == "BAD") {
  RCLCPP_INFO_LATCH(get_logger(), hw_state_latch, true, "hardware not ready yet");
 }
  if (state == "GOOD") {
  RCLCPP_INFO_LATCH(get_logger(), hw_state_latch, false, "System in good state, ready.");
 }
}

Yielding:

hardware not ready yet
// and finally system swaps over
System in good state, ready.
// and then back again
hardware not ready yet

I'd be willing to write this feature if someone could give me instructions on how it would be implemented in the rcutils framework, and if it would actually be merged.

I am not too sure on how to define the static/global that can be referenced in other functions.

@Barry-SH
Copy link

Currently, the log provides the function RCLCPP_INFO_ONCE(), but it doesn't fully meet your needs.
If you want to implement it, you can refer to the implementation of RCLCPP_INFO_ONCE()

  • rclcpp/resource/logging.hpp.em
  • rcutils/resource/logging_macros.h.em

I'm curious why not implement it with code instead of relying on log functions? Implementing it by the user would also be simple.

@russkel
Copy link
Author

russkel commented Mar 19, 2024

Thanks.

I'm curious why not implement it with code instead of relying on log functions? Implementing it by the user would also be simple.

You could say that about all the log macros.

We have ended up implementing many "simple" things throughout our code base in relation to rclcpp functionality and it get tiring writing out the same boilerplate. I intend to follow the same approach here but I wanted to see if it was possible using log macros. If it can, then great I will create a PR, if not, it goes into our private libraries never seeing the light of day.

@Barry-SH
Copy link

I have no reason to oppose adding such a feature to the log function. It brings convenience to the users. Let's hear the opinions of other community members.

@fujitatomoya
Copy link
Collaborator

@russkel thanks for creating issue.

hardware not ready yet
// and finally system swaps over
System in good state, ready.
// and then back again
hardware not ready yet

i guess RCLCPP_INFO_ONCE would not be useful with above use case, that state gets back to the previous state.

maybe there are some constraints, but i would not use topics to share the hardware or sensor state.

instead,

  • using service, so that client (user) can know the state anytime they want. (client can ask it when needed, instead of waiting for the topics. this probably conserves the inter-process communication.)
  • or parameters? make it read only parameter and read it from the remote nodes, so that client can know the state. besides, using parameter events can be more useful to get the event once the parameter is changed on the device.

@russkel
Copy link
Author

russkel commented Mar 19, 2024

maybe there are some constraints, but i would not use topics to share the hardware or sensor state.

This was an example (and it's dealing with the data stream such as serial from a hardware sensor), this is pretty common where sensors will report their state at > 1Hz. There's no publishing in the example.

I imagine there's a few use cases outside of this example too.

@cottsay cottsay added the help wanted Extra attention is needed label Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants