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

ROS1/ROS2 Hybrid: delphi_srr_msgs #40

Merged
merged 2 commits into from
Nov 27, 2019
Merged

Conversation

JWhitleyWork
Copy link
Contributor

@JWhitleyWork JWhitleyWork commented Nov 15, 2019

This is the second AutonomouStuff message package which is being hybridized to be able to be built in both ROS1 Kinetic/Melodic and ROS2 Dashing. CI on this branch is now running all 3 environments.

I've included at least one member from each engineering team to make sure that everyone has a chance to give feedback and we don't merge this until we're sure that nothing will break. After the following tasks are complete and I have an accepted review from each team, we will merge this in tandem with the changes to the other nodes that it effects. Here are the items that I know of (currently) that need to be completed before this can be merged:

  • Update ros_delphi_srr driver to match message changes
  • Add and test bag migration rules
  • Test all changes on real sensor

If there are other items that need to be updated to coincide with this merge, please speak up and I'll add them to this list. Thanks!

mlemm99
mlemm99 previously approved these changes Nov 15, 2019
@JWhitleyWork
Copy link
Contributor Author

Sorry, @mlemm99 - Please review again. I forgot to add the BAG migration rules.

@mlemm99 mlemm99 self-requested a review November 15, 2019 17:47
mlemm99
mlemm99 previously approved these changes Nov 15, 2019
Daniel-Stanek
Daniel-Stanek previously approved these changes Nov 15, 2019
@icolwell-as
Copy link
Member

Looks like some conflicts need to be resolved.

@JWhitleyWork
Copy link
Contributor Author

@icolwell-as - Conflicts resolved. It's hard to keep all of them conflict-free when I have to partition each package off when CI runs to get it to pass before all packages have been ported.

Daniel-Stanek
Daniel-Stanek previously approved these changes Nov 18, 2019
mlemm99
mlemm99 previously approved these changes Nov 18, 2019
icolwell-as
icolwell-as previously approved these changes Nov 20, 2019
jilinzhouas
jilinzhouas previously approved these changes Nov 20, 2019
@JWhitleyWork JWhitleyWork merged commit 721a15d into master Nov 27, 2019
@JWhitleyWork JWhitleyWork deleted the maint/hybridize_delphi_srr branch November 27, 2019 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants