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

Finally add STALE #33

Merged
merged 1 commit into from
Apr 24, 2014
Merged

Finally add STALE #33

merged 1 commit into from
Apr 24, 2014

Conversation

trainman419
Copy link
Contributor

yep; it's going to break everything. We don't guarantee cross-version compatibility anyway, and this has been around long enough that it needs to get added to the message definition sooner or later.

yep; it's going to break everything. We don't guarantee cross-version compatibility anyway, and this has been around long enough that it needs to get added to the message definition sooner or later.
@chadrockey
Copy link
Member

+1 this is long overdue and shouldn't be too painful.

Can you add a migration rule to diagnostic_msgs in case someone has these bagged?

@tfoote
Copy link
Member

tfoote commented Apr 9, 2014

+1 with a bag migration rule

@trainman419
Copy link
Contributor Author

I tried to run the rule migration script and got this:

$ rosrun rosbag makerule.py DiagnosticStatus.saved DiagnosticStatusIndigo.bmr
The following migrations need to occur:
 * From: diagnostic_msgs/DiagnosticStatus [67d15a62edb26e9d52b0f0efa3ef9da7]
   To:   diagnostic_msgs/DiagnosticStatus [d0ce08bc6e5ba34c7754f563a9cabaf1]

No additional rule files needed to be generated.  DiagnosticStatusIndigo.bmr not created.

Did I miss an argument, or it it smart enough to figure out the migration without a rule?

@chadrockey
Copy link
Member

Its smart enough to match members one to one, and since package name, message name, and all members match, it will automatically migrate what's in a bag to what's in the ROS package path. The enum constants are just for developers and as integers, can't be migrated.

I think this is +1 to merge from me at this point.

@trainman419
Copy link
Contributor Author

@tfoote any thoughts on this? Is there a way I can generate a migration rule, or can we move forward without one?

@tfoote
Copy link
Member

tfoote commented Apr 24, 2014

I believe @chadrockey is right so we can merge this.

tfoote added a commit that referenced this pull request Apr 24, 2014
@tfoote tfoote merged commit 0943d37 into ros:indigo-devel Apr 24, 2014
@mikeferguson
Copy link
Contributor

I've added a note on the Indigo migration page about this, since it will most definitely break your robot_monitor or dashboard if you aren't paying close attention to which distro is running on each end (and there is no error/warning on the monitor/dashboard -- only in the robot logs)

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.

None yet

4 participants