ros / meta-ros

OpenEmbedded Layers for ROS 1 and ROS 2
MIT License
373 stars 246 forks source link

ROS2-Iron #1053

Closed codewithpassion closed 2 months ago

codewithpassion commented 9 months ago

As requested by @vmayoral on ROS Discourse.

I'm pretty new to Yocto/OE as well as ROS and this effort based on Mission Robotics needs for ROS2 Iron. I haven't tested all the ROS2 packages and there are most probably missing bits.

Please let me know what I can do to assist in improving this.

I also was part of todays call organized by Rob Wolley and will try to assist this working group in the future.

robwoolley commented 9 months ago

Thanks for this @codewithpassion this is really great. There was a specific template to the commits that Martin Jansa used to help tag what changes were being made. eg. {humble} cmake: Upgrade to 3.22

I'll make an attempt to go through your fixes in detail this week to get it merged in soon.

codewithpassion commented 9 months ago

Thanks @robwoolley

Can you elaborate on this?

There was a specific template to the commits that Martin Jansa used to help tag what changes were being made. eg. {humble} cmake: Upgrade to 3.22

Not sure if I understand what you're asking for?

codewithpassion commented 9 months ago

@robwoolley @vmayoral Is there any strategy on how to test things? Like a way to include all the generated recipes? At this point I've only compiled the things I needed for our project. But I'm sure there are more packages that need fixing.

robwoolley commented 9 months ago

Thanks @robwoolley

Can you elaborate on this?

There was a specific template to the commits that Martin Jansa used to help tag what changes were being made. eg. {humble} cmake: Upgrade to 3.22

Not sure if I understand what you're asking for?

Martin was the previous maintainer of meta-ros. He had a process for how he managed the chaos of having multiple Yocto Releases in each branch with multiple sublayers for each ROS Distro in each. He used tags in the commit subject to label with distros were being affected by a commit. For example:

{galactic}{rolling} smacc2: use new overrides syntax also in comment

I didn't really appreciate the system until I had to start cherry picking commits across different combinations.

Don't worry about having to change anything right now. I will make an attempt at it. I just wanted to let you know that I was reviewing your changes and you could expect to hear back from me soon.

If you are curious, I just pushed my maintainer notes based on the review I did of the git history. You can see the section on adding a new ROS Distro here: https://github.com/robwoolley/meta-ros-maintainer-notes/blob/main/newdistro.md

robwoolley commented 9 months ago

@robwoolley @vmayoral Is there any strategy on how to test things? Like a way to include all the generated recipes? At this point I've only compiled the things I needed for our project. But I'm sure there are more packages that need fixing.

I just published a draft of my notes from testing Milestone 17 with Herb: https://github.com/robwoolley/meta-ros-maintainer-notes/blob/main/testinstructions.md

Hope this helps! Let me know if you have any questions.

vmayoral commented 9 months ago

@codewithpassion there're conflicts in here. You may want to have a look at that.

codewithpassion commented 8 months ago

@codewithpassion there're conflicts in here. You may want to have a look at that.

Hi @vmayoral Thanks for your feedback. Could you give me a hint on where you see conflicts? Github shows no conflicts. Or what conflicts where you referring to?

vmayoral commented 8 months ago

@codewithpassion there're conflicts in here. You may want to have a look at that.

Hi @vmayoral Thanks for your feedback. Could you give me a hint on where you see conflicts? Github shows no conflicts. Or what conflicts where you referring to?

@codewithpassion, see message below "This branch cannot be rebased due to conflicts".

codewithpassion commented 8 months ago

@codewithpassion there're conflicts in here. You may want to have a look at that.

Hi @vmayoral Thanks for your feedback. Could you give me a hint on where you see conflicts? Github shows no conflicts. Or what conflicts where you referring to?

@codewithpassion, see message below "This branch cannot be rebased due to conflicts".

Thanks @vmayoral . Sadly, I can't seem to see this. Probably because I don't have write access. image

vmayoral commented 8 months ago

@codewithpassion there're conflicts in here. You may want to have a look at that.

Hi @vmayoral Thanks for your feedback. Could you give me a hint on where you see conflicts? Github shows no conflicts. Or what conflicts where you referring to?

@codewithpassion, see message below "This branch cannot be rebased due to conflicts".

Thanks @vmayoral . Sadly, I can't seem to see this. Probably because I don't have write access. image

Noted, thanks @codewithpassion. I went ahead and approved my previous review request but my view's still blocked by the conflicts:

imagen

I went through the files to try and locate the conflict but there's too many changes and I couldn't find it. Given this state, my suggestion would be to consider closing this PR and opening a new one after rebasing with langdale branch. It should be fairly easier for you to assess any conflicts locally before submitting again the PR.

robwoolley commented 2 months ago

I re-reviewed the PR to check for any changes that we missed. It looks like everything we talked about got merged in. Thanks, Dominik!