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

core: services: ping: minimise port duplication races #2354

Merged
merged 1 commit into from
Jan 29, 2024

Conversation

ES-Alexander
Copy link
Collaborator

@ES-Alexander ES-Alexander commented Jan 29, 2024

Hopefully fixes #1200 (or at least significantly reduces the likelihood of it).

Details:

  • ⚠️ Untested
    • should be testable at esalexander/blueos-core, :ping-fix-attempt (in the BlueOS Version Chooser)
  • Uses the approach I suggested here
  • It would be fully robust to also/instead regularly check for duplicated ports in the drivers (and restart them as necessary), but that seems excessive to me, and more involved to program and test
  • Ideally this code should also stop treating a port as being connected to once the connection is established, but that seems like extra effort for minimal gain (as far as I'm aware it's not important for the ports to be consistently the same, so as long as unique ones get chosen that aren't crazily far from the base port then it should be fine)
  • This code also has a bit of duplication in the handling of ping1d vs ping360, but I didn't want to deal with the type checker, and reducing the duplication may end up with slightly more code anyway

Copy link
Member

@Williangalvani Williangalvani left a comment

Choose a reason for hiding this comment

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

@Williangalvani Williangalvani merged commit 9d41f73 into bluerobotics:master Jan 29, 2024
7 checks passed
@ES-Alexander ES-Alexander deleted the ping-fix-attempt branch January 30, 2024 05:05
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.

bug: Ping service: Is creating pings with same port
3 participants