feat(core): add FeedHeartbeatWatchdog RPC#415
Conversation
|
@julianoes @JonasVautherin this is the proto API for the external heartbeat control discussed in mavlink/MAVSDK#2895. Implementation PR to follow in MAVSDK once the API shape is settled here. |
julianoes
left a comment
There was a problem hiding this comment.
So, if you set this once that means it's enabled, right?
|
No, calling it once doesn't enable it or latch it on. The feed just resets the watchdog timer. The client has to keep calling at its chosen rate (e.g. ~1 Hz), and each call pushes the expiry out. If the feeds stop, the timer expires and MAVSDK stops sending heartbeats. That's the fail-closed behavior: a crash, disconnect, or stalled control path lets the timer lapse so PX4 sees the component go offline and triggers the lost-link failsafe. Also worth clarifying: the watchdog has to be configured first. If it isn't enabled/configured, the feed call does nothing and MAVSDK keeps sending heartbeats normally on its own. So the default behavior is unchanged, and the feed only matters once you've opted into the watchdog. I've opened the implementation PR so the behavior is concrete: mavlink/MAVSDK#2905 |
julianoes
left a comment
There was a problem hiding this comment.
Aha, the config for it is not exposed via proto. Why not?
julianoes
left a comment
There was a problem hiding this comment.
Aha, the config for it is not exposed via proto. Why not?
|
Yeah, this one was a deliberate choice and I'm glad you flagged it, since it's worth talking through. I kept the timeout off the gRPC path because it's really a failsafe parameter rather than a normal runtime knob. The whole point of the watchdog is to catch a client that isn't there, so I got a little wary of letting a client lengthen or disable its own deadman at runtime, since that's the one thing that could quietly defeat the mechanism. So right now enable + timeout are startup config (CLI flag / Configuration), and only the feed is runtime. The way I'm thinking about it: once you've decided to run mavsdk_server with the watchdog, it's a fundamental property of how that server and its clients relate. The server's heartbeat now means "a client is present and healthy," and the client's job is to keep proving that by feeding. Letting a client turn that off in real time would be letting it opt out of being watched while staying connected, which is exactly the case the watchdog exists to catch. So I can't really come up with a use case for runtime disable that doesn't amount to losing the failsafe, because disabling it at runtime is losing the failsafe. The two are the same action. The startup approach has a nice side benefit too: clients don't have to configure anything. You launch mavsdk_server with the watchdog on and a fixed timeout, and clients just feed it. You're right that there's a gap though: a gRPC-only client (the language wrappers) can't turn the watchdog on at all right now. One way to close it would be a one-way enable over gRPC, where a client can turn the watchdog on but can't turn it off or change the timeout. That said, I'm not sure it earns its keep: it adds API surface and another state to reason about, and in practice I'd expect the watchdog to be set at startup anyway, so the runtime enable might be complexity without much real benefit. I'm inclined to keep config startup-only unless there's a wrapper use case that needs it. What do you think? Happy to add the enable-only path if you see a need for it, but my lean is to keep it simple. |
|
@tpayne-censystech can you be any more verbose? (sarcasm) We don't treat clients as adversarial, so we should expose enable and pet APIs to proto, in my opinion, otherwise that's just confusing. |
|
You still interested in this @tpayne-censystech? Sorry if I pissed you off but when I see a wall of text I assume it's AI. |
Ha, fair. Still interested, sorry for the delay, I got distracted with the July 4th holiday. And no offense taken. 😄 Makes sense, let me put the config on proto then. Proposal: a Work for you? |
|
Ok, sounds good. |
|
Can you do the MAVSDK side please? Happy to merge both PRs once it's done end to end. |
|
MAVSDK side is updated: mavlink/MAVSDK#2905. Implements SetHeartbeatWatchdogTimeout as discussed, positive value enables with that timeout, 0 disables. Still verifying end to end and will confirm here once that's done, but the review process could kick off in parallel if you're up for it. |
5763e49 to
9f4639e
Compare
Allow a client to keep MAVSDK's periodic heartbeats alive by feeding a deadman timer (configured in MAVSDK), so heartbeats stop when the client hangs or dies.
Add a gRPC path to enable or disable the heartbeat watchdog at runtime. timeout_s is a double in seconds: 0 disables the watchdog, and values greater than 0 must be at least 1 (timeouts between 0 and 1 are invalid).
9f4639e to
d019861
Compare
Adds a gRPC API for external control of MAVSDK heartbeat emission.
When enabled, this disables MAVSDK's internally generated HEARTBEAT sending and instead drives emission from the client via a runtime-configurable deadman timeout. The client refreshes within the timeout to keep heartbeats flowing; if it crashes, disconnects, or stops servicing the path, MAVSDK stops sending, so PX4 sees the GCS/companion component go offline and triggers the configured lost-link failsafe (e.g. RTL). MAVSDK continues to own heartbeat content (sysid, compid, mav_type, etc.); the client only controls whether emission continues. Default behavior is unchanged.
This is the proto/API definition only. The core implementation will follow in a separate PR against mavlink/MAVSDK.
Refs mavlink/MAVSDK#2895
Contributed by Tyler Payne on behalf of Censys Technologies, Inc.