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

Jazzy Jalisco #337

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft

Jazzy Jalisco #337

wants to merge 14 commits into from

Conversation

jimmy-mcelwain
Copy link
Collaborator

See #333. This updates MotoROS2 to make it compatible with Jazzy Jalisco, the new long term support release of ROS2.

@jimmy-mcelwain jimmy-mcelwain marked this pull request as draft November 25, 2024 14:25
@ted-miller ted-miller added this to the 0.2.0 milestone Nov 25, 2024
src/CommunicationExecutor.c Show resolved Hide resolved
src/MotoROS.h Outdated Show resolved Hide resolved
@ted-miller ted-miller linked an issue Nov 25, 2024 that may be closed by this pull request
@ted-miller
Copy link
Collaborator

This also needs updates to the readme

@jimmy-mcelwain
Copy link
Collaborator Author

This also needs updates to the readme

I can do that. Do we want to replace the examples in the README from Humble->Jazzy? Since Jazzy is the newest long term support? I know that we're still supporting Humble, but I just figured that we'll want to point new users to install Jazzy instead. Or do we want to keep Humble as the example distro for some time/does it not matter?

@ted-miller
Copy link
Collaborator

Do we want to replace the examples in the README from Humble->Jazzy?

I think so.

@jimmy-mcelwain
Copy link
Collaborator Author

jimmy-mcelwain commented Dec 6, 2024

I added Jazzy to the README. I replaced Humble with Jazzy in the example commands. I did not remove any references to Foxy or Galactic. I also removed a dead link to ROS Answers. I see discussion here that an archive of ROS Answers is being created. I can add back the link if that is later made available.

Copy link
Collaborator

@ted-miller ted-miller left a comment

Choose a reason for hiding this comment

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

Couple humble references remain:

motoros2/README.md

Lines 162 to 163 in 0dcf5a6

$ md5sum -b mr2_yrc1_h.out
e2d088b765a0bfed501aa213a1be1de0 mr2_yrc1_h.out

source /opt/ros/humble/setup.bash

Also, this table needs to be updated:

motoros2/README.md

Lines 169 to 179 in 0dcf5a6

|**Controller** |**ROS 2 Version** | **File** |**Version** | **MD5 hash** |
|:--------------|:-----------------|:------------------|:-----------|:-----------------------------------|
| DX200 | Foxy | `mr2_dx2_f.out` | `0.1.3` | `a9a9e10403f726062c25d97654fad316` |
| DX200 | Galactic | `mr2_dx2_g.out` | `0.1.3` | `e8db7512215da240b28b985f2f2af98b` |
| DX200 | Humble | `mr2_dx2_h.out` | `0.1.3` | `611bda537655cf8a60d85600da6043f4` |
| YRC1000 | Foxy | `mr2_yrc1_f.out` | `0.1.3` | `84bfb44e2043372127d9dfc1157a79b5` |
| YRC1000 | Galactic | `mr2_yrc1_g.out` | `0.1.3` | `866e090b6c724429ce03117712c951f4` |
| YRC1000 | Humble | `mr2_yrc1_h.out` | `0.1.3` | `e2d088b765a0bfed501aa213a1be1de0` |
| YRC1000micro | Foxy | `mr2_yrc1m_f.out` | `0.1.3` | `027e77b427a212aa63e5d7962d48ad92` |
| YRC1000micro | Galactic | `mr2_yrc1m_g.out` | `0.1.3` | `042d753a7729784fec8c5c23bef3e685` |
| YRC1000micro | Humble | `mr2_yrc1m_h.out` | `0.1.3` | `c0e61adbf5bf6fd6a734211f15bb0f0a` |

README.md Show resolved Hide resolved
@jimmy-mcelwain
Copy link
Collaborator Author

I added some more humble->jazzy swaps in the documentation. I don't know how I missed so many the first time around. We will still have to update the table with the new versions/hashes once they are ready.

.reuse/dep5 Outdated Show resolved Hide resolved
Copy link
Collaborator

@gavanderhoorn gavanderhoorn left a comment

Choose a reason for hiding this comment

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

Round of comments

doc/build_from_source.md Outdated Show resolved Hide resolved
src/CommunicationExecutor.c Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/MotoROS2_AllControllers.vcxproj.filters Outdated Show resolved Hide resolved
@jimmy-mcelwain
Copy link
Collaborator Author

I've addressed those changes. The project filters are particularly annoying to deal with, visual studio likes to change things in the background.

@gavanderhoorn
Copy link
Collaborator

@jimmy-mcelwain: the PR adding Iron support (#303) added these lines to the CommunicationExecutor.c. I was a bit surprised those were necessary in the first place, but I don't see them in this PR anymore.

Are they no longer needed? Were we doing something wrong previously? Or is this a difference between Iron and Jazzy (in our favour for once)?

(the Arduino examples also don't seem to include them: Iron version, Jazzy version)

@jimmy-mcelwain
Copy link
Collaborator Author

jimmy-mcelwain commented Dec 13, 2024

@jimmy-mcelwain: the PR adding Iron support (#303) added these lines to the CommunicationExecutor.c. I was a bit surprised those were necessary in the first place, but I don't see them in this PR anymore.

Are they no longer needed? Were we doing something wrong previously? Or is this a difference between Iron and Jazzy (in our favour for once)?

They are no longer needed. It is a difference between Iron and Jazzy in our favor. In Jazzy, the node_type_cache_init and node_type_cache_fini is all handled in the background for micro-ROS rcl. However, in Iron, it is not handled in the background. I don't know why the init/fini were manually patched out in Iron, but not in Jazzy.

I assume that the Arduino never includes them because the node_type_cache and all of the relevant functions only exist when RCL_MICROROS_COMPLETE_IMPL is set.

@jimmy-mcelwain
Copy link
Collaborator Author

jimmy-mcelwain commented Dec 13, 2024

What do we still have to do for release? I think that this repo should be good, except for we could clean up the commit history/rebase and update the table with new hashes. But first Yaskawa-Global/micro_ros_motoplus#11 needs to be reviewed.

@ted-miller
Copy link
Collaborator

What do we still have to do for release?

This passes all my testing. We just need to finish merging the back-end stuff.

update the table with new hashes

Not necessary until we make the 0.2.0 release.

README.md Outdated Show resolved Hide resolved
libyaml_vendor being installed by ament_cmake_vendor_package (a new change in jazzy) puts it in a different location than previously.
rclc_timre_init_default was replaced with rclc_timer_init_default2 in jazzy, which has a different set of parameters. The old function is now deprecated, this change gets rid of the message.
Also change the distro used in examples from Humble to Jazzy, and remove dead links (ROS Answers)
jimmy-mcelwain and others added 7 commits January 13, 2025 09:35
The buildscript has been changed so that vendor packages will have their name in the include path, matching non-vendor packages.
Distributions older than Jazzy place vendor packages with the rest of the source files, but Jazzy places them in the opt directory, so the build script places the include files somewhere else.
A link to a ROS Answers thread was removed after ROS Answers was taken offline, but now the archive is up so we can add the old link back.

Co-authored-by: G.A. vd. Hoorn <[email protected]>
@gavanderhoorn
Copy link
Collaborator

thanks for rebasing @jimmy-mcelwain.

This now builds because conditionals were added in ee58dda and 7facc1d. At least this introduced no regressions.

@gavanderhoorn
Copy link
Collaborator

gavanderhoorn commented Jan 13, 2025

(pre)released libmicroros Jazzy and updated CI here to build against it (911b2c5).

Should succeed, but let's see.


Edit: made a mistake. Will fix in a bit.

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.

Support Jazzy Jalisco
3 participants