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

Video: Basic implementation of MAVLink camera protocol #217

Merged
merged 1 commit into from
May 22, 2024

Conversation

ddd999
Copy link

@ddd999 ddd999 commented Apr 10, 2024

Adds a MAVLink heartbeat output for the attached camera device, and adds the ability to send CAMERA_INFORMATION messages when requested.

Also includes improvements for better handling undefined parameters for sendCommandAck() and sendHeartbeat()

This completes a basic implementation of feature request #169.

@stephendade
Copy link
Owner

Thanks! I should have some time in the next few days to review.

@stephendade
Copy link
Owner

Only major issue I'm seeing is that the "Enable camera heartbeats" (and streaming is active) checkbox doesn't remain ticked if I refresh the Video Streaming page. Perhaps the value isn't getting passed through correctly?

@ddd999
Copy link
Author

ddd999 commented May 5, 2024

I am seeing the same thing but really can't figure out why. As far as I can tell, the camera heartbeat setting is being saved and retrieved the exact same way as the timestamp setting, but only the heartbeat setting isn't getting retrieved properly. If you have time to take a closer look at the code, another pair of eyes would be much appreciated.

server/index.js Outdated
@@ -427,6 +449,7 @@ app.get('/api/videodevices', (req, res) => {
error: null,
fps: fps,
FPSMax: FPSMax,
useCameraHeartbeat,
Copy link
Owner

Choose a reason for hiding this comment

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

enableCameraHeartbeat: useCameraHeartbeat,

Copy link
Author

Choose a reason for hiding this comment

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

Well that is an embarrassing thing to miss! Thanks for finding it.

@stephendade
Copy link
Owner

stephendade commented May 8, 2024

I've added in a comment for fixing the non-appearing-checkbox.

One other thing, you'll need to move the Video streaming service to below the UDP IP/port options in ./src/video.js, as the user may get confused when RTP mode is selected.

@ddd999 ddd999 force-pushed the camerainfo branch 2 times, most recently from 3729012 to 63a0c82 Compare May 13, 2024 05:08
@ddd999
Copy link
Author

ddd999 commented May 13, 2024

I've added in a comment for fixing the non-appearing-checkbox.

One other thing, you'll need to move the Video streaming service to below the UDP IP/port options in ./src/video.js, as the user may get confused when RTP mode is selected.

Both of these things should be fixed in the latest commit. Thanks for your help!

@stephendade
Copy link
Owner

Found another issue - the "Video source IP Address" doesn't appear to saved. It goes back to 127.0.0.1 whenever I refresh the page (with streaming active).

Adds a MAVLink heartbeat output for the attached camera device, and adds the ability to send CAMERA_INFORMATION messages when requested.

Also includes improvements for better handling undefined parameters for sendCommandAck() and sendHeartbeat()

This completes a basic implementation of feature request stephendade#169.
@ddd999
Copy link
Author

ddd999 commented May 20, 2024

Found another issue - the "Video source IP Address" doesn't appear to saved. It goes back to 127.0.0.1 whenever I refresh the page (with streaming active).

That should be fixed now.

There are a couple of other slight changes in the latest commit, to comply with the expected behaviour based on the MAVLink documentation.

When RTP streaming is selected:

  • The video source IP box below the "Enable camera heartbeats" checkbox is hidden on the Video page, since it's not needed
  • The URI field in the VIDEO_STREAM_INFORMATION message will only contain the destination UDP port

I noticed that Mission Planner recognizes the VIDEO_STREAM_INFORMATION message, and asks if you want to connect to the RTSP stream, which worked correctly when I tried it. MP seems to only ask this the first time it receives a message, though. It also doesn't appear to do this when the stream type is RTP.

@stephendade
Copy link
Owner

Looks good! Merging...

@stephendade stephendade merged commit df4ff6a into stephendade:master May 22, 2024
9 checks passed
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.

2 participants