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.

Sign up for GitHub

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

Jump to bottom

Update BatteryState.msg with temperatures #140

Merged
merged 1 commit into from
Mar 16, 2020
Merged

Update BatteryState.msg with temperatures #140

merged 1 commit into from
Mar 16, 2020

Conversation

reinzor
Copy link
Contributor

@reinzor reinzor commented Dec 7, 2018 •
edited
Loading

Resolves #139

@reinzor
Copy link
Contributor Author

reinzor commented Jan 17, 2019

ping

@tfoote
Copy link
Member

tfoote commented Jan 17, 2019

This looks like a reasonable addition. Changing a core message like this is not something that we can do during a release cycle so we will need to target Noetic. And it would be good to make sure to clearly document any migration requirements for existing users, both for requirements for their code base as well as bag migration rules for logged data.

clalancette
clalancette approved these changes Feb 25, 2019
View reviewed changes
Copy link

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

This looks good to me, provided that we create a new "noetic-devel" branch and target that. We also want this in ROS 2; I'll make a separate PR for that.

clalancette added a commit to ros2/common_interfaces that referenced this pull request Feb 25, 2019
@clalancette
Add temperature to the BatteryState message.
895acfb 
Essentially a cherry-pick of ros/common_msgs#140
onto ROS 2.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette clalancette mentioned this pull request Feb 25, 2019
Merged
clalancette added a commit to ros2/common_interfaces that referenced this pull request Feb 25, 2019
@clalancette
Add temperature to the BatteryState message. (#68)
8fa6936 
Essentially a cherry-pick of ros/common_msgs#140
onto ROS 2.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@reinzor
Copy link
Contributor Author

reinzor commented Mar 13, 2020

Can this be closed or do you still require this?

@clalancette
Copy link

I think this is a good idea to do; thanks for pinging. Now that we are heading towards noetic, I'd create a new branch (noetic-devel), retarget this there, and then merge it. @tfoote any thoughts? If not, I'll just go ahead and do it.

@clalancette clalancette changed the base branch from jade-devel to noetic-devel March 16, 2020 13:57
@clalancette
Copy link

All right, I've created a new noetic-devel branch, then retargeted this PR at that branch. I'm going to go ahead and merge it now. We'll also need an update to https://github.com/ros/rosdistro/blob/master/noetic/distribution.yaml to update the branches there, which I'll do shortly.

@clalancette clalancette merged commit 4804aa3 into ros:noetic-devel Mar 16, 2020
@reinzor
Copy link
Contributor Author

reinzor commented Mar 16, 2020

Thanks!

@tfoote
Copy link
Member

tfoote commented Mar 18, 2020

Branch change upstream for cross reference. ros/rosdistro#24094

@dirk-thomas
Copy link
Member

This has not been released into Noetic. Doing so after the release breaks the interface.

@gavanderhoorn
Copy link

Just to keep things connected: this PR has come up on ROS Answers: How to receive standard message that has changed format in noetic?.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clalancette clalancette clalancette approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add cell_temperature to sensor_msgs/BatteryState
5 participants
@reinzor @tfoote @clalancette @dirk-thomas @gavanderhoorn