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 #6578 Devices with Duplicate Names Cause Unexpected Behavior #6579

Conversation

CoolSpy3
Copy link
Contributor

@CoolSpy3 CoolSpy3 commented Jul 10, 2024

Description
Updates the controller api to create devices using their device tags instead of their names.

Note: I added a test for this behavior into other_api because that's where I saw the only other C++ test. If it should be moved to api, let me know, and I can make that change.

Related Issues
This pull-request fixes issue #6578.

Tasks
Add the list of tasks of this PR.

  • Fix the bug in the C++ controller API
  • Propagate necessary changes to other language APIs
  • Update the documentation
  • Update the changelog

Documentation
Robot.md.

@CoolSpy3 CoolSpy3 added the test suite Start the test suite label Jul 10, 2024
@CoolSpy3 CoolSpy3 linked an issue Jul 10, 2024 that may be closed by this pull request
@CoolSpy3 CoolSpy3 marked this pull request as ready for review July 10, 2024 01:36
@CoolSpy3 CoolSpy3 requested a review from a team as a code owner July 10, 2024 01:36
Copy link
Member

@omichel omichel left a comment

Choose a reason for hiding this comment

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

Thank you! I just have a minor suggestion.
Also, I believe you forgot to update the change log.

Copy link
Member

@omichel omichel left a comment

Choose a reason for hiding this comment

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

Thanks. Almost there.

docs/reference/changelog-r2024.md Outdated Show resolved Hide resolved
Co-authored-by: Olivier Michel <[email protected]>
@omichel omichel self-requested a review July 10, 2024 06:36
omichel
omichel previously approved these changes Jul 10, 2024
Copy link
Member

@omichel omichel left a comment

Choose a reason for hiding this comment

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

All right, thank you.

@omichel
Copy link
Member

omichel commented Jul 10, 2024

Note: I added a test for this behavior into other_api because that's where I saw the only other C++ test. If it should be moved to api, let me know, and I can make that change.

I believe that's fine this way.

omichel
omichel previously approved these changes Jul 10, 2024
@CoolSpy3
Copy link
Contributor Author

CoolSpy3 commented Jul 10, 2024

Alright, I've fixed the test suite (for real this time). I'm still blocked from merging because the macos source check is required, so you can either override that check, or we can wait for #6580 to be completed.

@omichel omichel merged commit a01ffad into cyberbotics:master Jul 10, 2024
17 of 20 checks passed
@CoolSpy3 CoolSpy3 deleted the fix-devices-with-duplicate-names-cause-unexpected-behavior branch July 29, 2024 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test suite Start the test suite
Development

Successfully merging this pull request may close these issues.

Devices with duplicate names cause unexpected behavior
2 participants