Skip to content

Comments

Send heartbeat responses#75

Merged
loukylor merged 4 commits intomainfrom
uellenberg/hb-res
Oct 25, 2024
Merged

Send heartbeat responses#75
loukylor merged 4 commits intomainfrom
uellenberg/hb-res

Conversation

@uellenberg
Copy link
Collaborator

This sends heartbeat responses so that mission control can detect disconnection and reconnect automatically.

Copy link
Member

@loukylor loukylor left a comment

Choose a reason for hiding this comment

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

Services would probably work better here (rather than a pub and sub) since you can send back a response after receiving a request. You can look at #72 or the arm code to see an example of services. Here's the relevant ROS docs

@uellenberg
Copy link
Collaborator Author

Services would probably work better here (rather than a pub and sub) since you can send back a response after receiving a request. You can look at #72 or the arm code to see an example of services. Here's the relevant ROS docs

@loukylor I'll take a look at it tomorrow. Ideally, this is just a temporary solution for making mission control able to reconnect to the websocket automatically. What I really want to do is use websocket ping/pong control frames, since they'll end up using significantly less data. So, I'll probably modify all this code anyway - I just have to figure out how to listen to pings/pongs through rosbridge_server (which may require modifying it).

@loukylor
Copy link
Member

Ideally we don't have to modify rosbridge_server as that'd greatly increase the effort to maintain it. Is the ack really taking up that much bandwidth in the first place? If it isn't, we can just stick to pub/subs or services.

@adamseth2
Copy link
Contributor

Personally, I think sub/pub right now is fine, and can always overhaul it as this is a nice feature. I am also not too worried about bandwidth for this one as it is just sending the time. Right now though, it doesn't work for me for some reason though.

@uellenberg
Copy link
Collaborator Author

Nevermind, I've learned that for some reason browsers don't expose ping/pong control frames, so we won't be able to use them. I'll look into services - one issue with pub/sub is that ping/pong isn't directed towards a single client, which potentially breaks things if we ever want to support multiple clients.

@uellenberg
Copy link
Collaborator Author

Services would probably work better here (rather than a pub and sub) since you can send back a response after receiving a request. You can look at #72 or the arm code to see an example of services. Here's the relevant ROS docs

@loukylor ROSLib services don't appear to support CBOR, and ends up sending more data anyway. I agree that a service model makes more sense, but I'm not sure what real benefits it brings. Switching to a service will just make the message length longer, and I'm not sure whether that's a good idea for something that's sent as frequently as a heartbeat.

@loukylor
Copy link
Member

While I still disagree that services may affect bandwidth, since a single service call per second would not even be on the order of 100bytes per second, since this is already implemented I am okay with merging this. I will ensure formatting is right and review functionality then approve.

@loukylor
Copy link
Member

Has this been tested?

@uellenberg
Copy link
Collaborator Author

Has this been tested?

@loukylor Yes, Adam and I tested it yesterday.

@loukylor loukylor merged commit 0d595d4 into main Oct 25, 2024
@uellenberg uellenberg deleted the uellenberg/hb-res branch October 25, 2024 02:56
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