Skip to content

Navigation Menu

Sign in
Sign up

Do not generate s500 since it conflicts with Ping1D Message definition - #168

Open
patrickelectric wants to merge 1 commit into
bluerobotics:master from
patrickelectric:fix-ping
Open

Do not generate s500 since it conflicts with Ping1D Message definition #168
patrickelectric wants to merge 1 commit into
bluerobotics:master from
patrickelectric:fix-ping

Conversation

@patrickelectric

@patrickelectric patrickelectric commented Jan 17, 2026

Copy link
Copy Markdown
Member

No description provided.

Conflict with Ping1D definition of message for set speed of sound, where they use same id by different units
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Comment thread brping/pingmessage.py
definitions.PING360_DEVICE_DATA,
definitions.PING360_AUTO_DEVICE_DATA,
definitions.SURVEYOR240_ATOF_POINT_DATA,
definitions.SURVEYOR240_YZ_POINT_DATA,

@joaoantoniocardoso joaoantoniocardoso Jan 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

was this removal intentional?

@patrickelectric patrickelectric Jan 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nops, thanks for noticing it!

joaoantoniocardoso reacted with thumbs up emoji

@ES-Alexander ES-Alexander left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't like that we're removing library support for a publicly used message set, but I'm also not sure what the alternative is if it breaks the library / causes conflicts for use-cases that were established before that message set was added...

It may be preferable to allow message sets to overwrite/extend other messages, but if I'm understanding correctly there are issues with doing that currently, which makes this a necessary evil fix for an issue that will ideally have a better solution longer term.

Please do fix the point @joaoantoniocardoso raised before merging though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joaoantoniocardoso joaoantoniocardoso joaoantoniocardoso left review comments
@ES-Alexander ES-Alexander ES-Alexander approved these changes
@joaomariolago joaomariolago Awaiting requested review from joaomariolago

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /