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.

Sign up for GitHub

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

Jump to bottom

Add more camera and camera server functionality #320

Merged
merged 11 commits into from
Dec 24, 2023
Merged

Add more camera and camera server functionality #320

merged 11 commits into from
Dec 24, 2023

Conversation

tbago
Copy link
Contributor

@tbago tbago commented Jun 17, 2023 •
edited
Loading

1、add start stop video streaming in camera server, add streaming id in camera side (the camera can support multi video stream, so the stream id is useful).
2、add subscribe camera mode;
3、add subscribe storage information and capture stauts;

@tbago
Copy link
Contributor Author

tbago commented Jun 17, 2023

In the pre issue, we have alreay have some discuss.

1、For now I think the duplication of proto is ok too;
2、In the camera server side, I just implement the MAVLINK_MSG_ID_CAMERA_CAPTURE_STATUS MAVLINK_MSG_ID_STORAGE_INFORMATION in separeate message. two separate message is easy to implement but is not the same in the camera side.

@tbago
Copy link
Contributor Author

tbago commented Jun 17, 2023

If the following commit is ok, then I will continue to add the following funcitons:
1、reset_settings and format storage;

2、set video streaming info in camera server to make the subscirbe video streaming working, (support multi video streaming);

3、Add support manual set the camera definition file outside. I know in the camera already support http download inside.
But if I want to support use mavlink ftp to download the definition file, I need to tell the MAVSDK the download file location. So I prefer to download the file in client side, and just pass the definition file data to MAVSDK. I think use the http download inside the MAVSDK will make it easy to use. So one more suggestion, if the mavlink ftp can support download the file in memory maybe more better.
Sorry for my poor english, my suggestion is there are two way to implement the set camera definition file(support both http and mavlink ftp)
1、best way, the mavlink ftp can support download file in memory, and the MAVSDK user will not care about the download method;
2、better way, support manual set camera definition data, and the MAVSDK user can use anyway they like to download the definition file.

julianoes
julianoes reviewed Jun 17, 2023
View reviewed changes
Copy link
Collaborator

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

Thanks, see comment regarding units.

protos/camera_server/camera_server.proto Outdated
}
float image_interval = 1; // Image capture interval (in s)
float recording_time_s = 2; // Elapsed time since recording started (in s)
float available_capacity = 3; // Available storage capacity. (in MiB)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest to add the units like the other fields:

Suggested change
float available_capacity = 3; // Available storage capacity. (in MiB)
float available_capacity_mib = 3; // Available storage capacity. (in MiB)

And in other places too.

JonasVautherin
JonasVautherin previously approved these changes Jun 18, 2023
View reviewed changes
@tbago
Copy link
Contributor Author

tbago commented Jun 24, 2023

This PR is all about the basic camera function.
For more function, I will create new PR. So for now the PR is complete.

julianoes
julianoes requested changes Jun 24, 2023
View reviewed changes
Copy link
Collaborator

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

I think we need to rethink the API. See inline comment.

protos/camera_server/camera_server.proto Show resolved Hide resolved
protos/camera_server/camera_server.proto

message SubscribeStopVideoStreamingRequest {}
message StopVideoStreamingResponse {
int32 stream_id = 1; // video stream id
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, should it have a result?

protos/camera_server/camera_server.proto Show resolved Hide resolved
protos/camera_server/camera_server.proto Outdated Show resolved Hide resolved
@tbago tbago mentioned this pull request Jun 24, 2023
Merged
@julianoes julianoes mentioned this pull request Jun 28, 2023
Merged
@julianoes
Copy link
Collaborator

@tbago just letting you know: I'm now looking into it and will be working on it to get it merged as soon as possible.

@tbago
Copy link
Contributor Author

tbago commented Nov 15, 2023

Ok, I would like to contribute to the open source community, so what can I do for now?

@julianoes
Copy link
Collaborator

julianoes commented Nov 15, 2023 •
edited
Loading

If it's ok I will rebase and force push your branch here with the latest changes I have been doing and we continue from there?

@julianoes julianoes force-pushed the add_more_camera_function branch from 1981814 to 87fbac0 Compare December 17, 2023 21:29
@julianoes
Copy link
Collaborator

@tbago I've force pushed it. I'll also force push the MAVSDK PR: #320
And then work on merging it.

@julianoes julianoes mentioned this pull request Dec 17, 2023
Closed
tbago and others added 11 commits December 24, 2023 20:55
@tbago @julianoes
camera server : add start stop video subscribe (mavlink#318)
b953379
@julianoes
camera_server: typos
b97cf12 
Signed-off-by: Julian Oes <[email protected]>
@tbago @julianoes
camera server: add start stop video streaming, add stream id in camer…
6c25079 
…a side
@tbago @julianoes
camera server : add subscribe camera mode
032a94b
@tbago @julianoes
camera server: add storage information and capture stauts subscribe
6857ed6
@tbago @julianoes
add unit on image interval and available capacity
5fc6c24
@tbago @julianoes
storage information : add unit on read speed and write speed
8ed884f
@tbago @julianoes
camera : add format storage and reset settings function
6bc45ef 
  * add storage id in camera side, and then the camera can support multi
    storage format.
@tbago @julianoes
camera server: add respond for all camera server command
37e64ba
@julianoes
camera_server: add video stream information
a7c8dde 
Signed-off-by: Julian Oes <[email protected]>
@julianoes
param_server: add subscription for changed params
8184c04 
Signed-off-by: Julian Oes <[email protected]>
@julianoes julianoes force-pushed the add_more_camera_function branch from 87fbac0 to 8184c04 Compare December 24, 2023 07:55
@julianoes julianoes changed the title Add more camera function Add more camera and camera server functionality Dec 24, 2023
@julianoes julianoes merged commit a10b832 into mavlink:main Dec 24, 2023
3 checks passed
@tbago tbago deleted the add_more_camera_function branch February 22, 2024 08:19
@julianoes julianoes mentioned this pull request May 27, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julianoes julianoes julianoes approved these changes

@JonasVautherin JonasVautherin JonasVautherin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tbago @julianoes @JonasVautherin