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

laptop_battery_monitor: Cannot run multiple instances for diagnostics #3

Open
mitchellwills opened this issue Nov 11, 2014 · 4 comments

Comments

@mitchellwills
Copy link
Contributor

Currently you cannot run multiple instances of laptop_battery_monitor for diagnostics because every instance of the node assigns the same name to the diagnostics status message. It might make sense to switch to either use the hostname or use diagnostic updater, which puts the node name in the status name.

@bit-pirate
Copy link
Member

You have multiple batteries in your system?

@mitchellwills
Copy link
Contributor Author

While it is definitely possible to have multiple batteries, I don't. I was more talking about a system with multiple laptops in it (for example a turtlebot with a laptop on it that is controlled by another laptop).

@jihoonl
Copy link
Member

jihoonl commented Jan 6, 2015

@mitchellwills Is this resolved by #2?

@mitchellwills
Copy link
Contributor Author

No there is an issue here: https://github.com/ros-drivers/linux_peripheral_interfaces/blob/master/laptop_battery_monitor/scripts/laptop_battery.py#L203.
Currently the diagnostic status name is hard coded to "Laptop Battery". While this works fine with one instance of the node it, running multiple instances of the node will result in the messages from each node being treated as the same diagnostic status. You can test this by running two instances of the node (with different names), diagnostic_aggregator and rqt_robot_monitor.
There are two ways to solve this I think. One is to make a name parameter that defaults to the current string.
The second solution would be to prefix the name with the node name. This is what diagnostic_updater does.
I think that the second solution is the better solution; however, it is technically a breaking change as it may require users to update their diagnostic_aggregator config.

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

No branches or pull requests

3 participants