Skip to content

stores: video: Send MAVLink message when starting or stopping recording - #2420

Closed
patrickelectric wants to merge 1 commit into
bluerobotics:masterfrom
patrickelectric:record
Closed

stores: video: Send MAVLink message when starting or stopping recording#2420
patrickelectric wants to merge 1 commit into
bluerobotics:masterfrom
patrickelectric:record

Conversation

@patrickelectric

@patrickelectric patrickelectric commented Feb 23, 2026

Copy link
Copy Markdown
Member

Fix #734

Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>

@rafaellehmkuhl rafaellehmkuhl left a comment

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.

I would ask you to use a try/catch block in this case instead of the chained catch. That's because the sendCommandLong method is a deeply nested method and if any of its internal calls (or from its nested methods) throws synchronously before returning the promise (a.k.a any implementation error there), we would not continue the execution (which I understand do not exist right now, besides the alert pushing, but end up being added latter).

It ends up being a one line change but with the benefit of not having to closely monitor changes here in the future.

Comment thread src/stores/video.ts

activeStreams.value[streamName]!.mediaRecorder!.stop()

mainVehicle.value?.sendCommandLong(MavCmd.MAV_CMD_VIDEO_STOP_CAPTURE, 0, 0).catch((error) => {

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.

If this isn't going to handle individual cameras and streams, it should at least have a TODO comment about doing so. At the moment starting or stopping one recording at the surface would start or stop all the recordings in the vehicle, which is unlikely to be what the user expects (even if it's at least closer to their expectations than the current approach of always recording everything while armed).

@ES-Alexander ES-Alexander added the docs-needed Change needs to be documented label Mar 27, 2026
@rafaellehmkuhl

Copy link
Copy Markdown
Member

Closing in favor of #2839.

@ES-Alexander ES-Alexander removed the docs-needed Change needs to be documented label Aug 17, 2026
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.

Record button: Add an option for cockpit to ask camera to record locally (on vehicle)

3 participants