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

Fix LCM traffic on macOS Sequoia #303

Merged
merged 1 commit into from
Feb 7, 2025
Merged

Fix LCM traffic on macOS Sequoia #303

merged 1 commit into from
Feb 7, 2025

Conversation

tyler-yankee
Copy link
Contributor

@tyler-yankee tyler-yankee commented Feb 3, 2025

Addresses #22322. Network config changes don't persist on the CI images, so this change has to go in a setup script that is run each time Jenkins deploys a VM. Currently, all other macOS setup scripts only run once, or they don't run on both provisioned and unprovisioned images. Running this command is required on Sequoia and harmless on Sonoma.


This change is Reviewable

@tyler-yankee tyler-yankee added the blocked Not ready to merge or review label Feb 3, 2025
@tyler-yankee
Copy link
Contributor Author

See #22572 for re-enabling the unit test that was previously failing.

Builds on mac-arm-sequoia-clang-bazel-experimental-everything-release and mac-arm-sequoia-unprovisioned-clang-bazel-experimental-everything-release, which were run using this PR and the PR on drake-ci, are both green.

@tyler-yankee tyler-yankee removed the blocked Not ready to merge or review label Feb 4, 2025
@tyler-yankee
Copy link
Contributor Author

Perhaps both +@rpoyner-tri and +@jwnimmer-tri would like to look at this fix. What are your thoughts on updating documentation, especially user-facing (in case anyone else has this issue on macOS)?

Copy link
Contributor

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

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

:lgtm:

Yes, I think we should update the documentation. Perhaps a new entry in https://drake.mit.edu/troubleshooting.html and then the class overview for DrakeLcm can link to that new section.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @rpoyner-tri)

Copy link
Contributor

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @tyler-yankee)

@jwnimmer-tri
Copy link
Contributor

@tyler-yankee FYI for drake-ci, we usually let Kitware actually hit the merge button when you're ready, rather than the reviewers merging.

@tyler-yankee
Copy link
Contributor Author

Sounds good, I'm going to wait until we find a fix for #22587 before merging in anything new.

@tyler-yankee tyler-yankee merged commit 70e4326 into main Feb 7, 2025
1 check passed
@tyler-yankee tyler-yankee deleted the lcm-sequoia branch February 7, 2025 20:42
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.

3 participants