common: Add camera instance field to MAV_CMD_IMAGE_START/STOP_CAPTURE

[khanasif786]

I am working on enhancing camera and gimbal interface in Ardupilot, We are adding support of multiple cameras so a camera instance field is required for telling the camera in which we want to start and stop capturing images.

[rmackay9]

@hamishwillee @Davidsastresas @julianoes,

How do you guys feel about this?

I suspect that this first field has actually been reserved for this purpose similar to the MAV_CMD_DO_CONTROL_VIDEO message's "Camera ID" field.

I think what we should clarify whether the meanings (if any) for "-1" and "0". So for example, "-1" could be "all", "0" could be primary (?), 1 could be 1st camera, 2 could be the 2nd camera, etc. I guess it should be similar to the gimbal device id as it appears the CAMERA_INFORMATION message.

[hamishwillee]

This is not the first time the issue has come up. Another suggestion was in #1749, which was to select the set of cameras for which subsequent commands apply. It was closed - not sure of the reason, but I do remember some of the concerns and suggestions.

My take on this is that specifying the ID makes sense but with some caveats:

• You can only specify all or one particular camera - this approach means you can't specify sets of cameras, 1 and 3 and 77 (say)
  • EDIT: Maybe you can - i.e. by calling MAV_CMD_IMAGE_START_CAPTURE multiple times. I suspect that would work for commands, but the mission protocol might assume that a second MAV_CMD_IMAGE_START_CAPTURE is an "override" of the previous capture command. Certainly if a new system has not been updates it will treat it as such.
• If MAV_CMD_IMAGE_START_CAPTURE (or stop) is sent as a command it goes to the specific system and component.
  • You would need to resolve how the ID works in if it is sent as a command - presumably it should be ignored.
  • The reserved value for param 1 is 0. This must still be zero for the "default case" otherwise you will get unintended behaviour if sent from a GCS that things 0 mean nothing.
• If MAV_CMD_IMAGE_START_CAPTURE is used in a mission, currently how it behaves in the case of multiple cameras is undefined in the spec, but I think "effectively defined" by routing behaviour.
  • Specifically I think that for this to work the command has to be sent EITHER with a broadcast component ID so that it turns on all cameras OR with the first default camera component ID and can only turn on that specific camera. The second approach is rubbish because component IDs should not imply "purpose".
  • Upshot - 0 has to mean "enable all cameras", at least in the mission case.
• Currently all addressing is via component ID in the command. How would a camera know that it was the primary camera, first camera, whatever? All they know about themselves is their component ID and system ID. It is best to avoid having to manually apply this so you'd need to either have a camera manager that assigns and ID or similar, or just use the component ID.
• Given that addressing is via a component ID, this means you can't have more than one camera direct-connected to the autopilot (same reasons as for gimbal).
• Cameras getting this won't know what to do with it without an update - i.e. in a mission you send this out. A recipient will get the message with broadcast id and know it can act on it. A current camera will act on it.
• There was discussion about addressing cameras via purpose - i.e. specify "capture on IR camera", "Capture on primary", "Capture on camera property". That requires integration work and a lot more thought.

I think this might work, given above caveats. The benefit is that things know their own ID and you can map the identity from camera information to component ID in a GCS (so nothing else needs to know the component ID in advance). Downside is that it will require update to firmware on cameras to understand that they need to do this in missions.

     <param index="1" label="target_component" minValue="0" maxValue="255" increment="1">Target camera component ID when sent in missions. 0: all cameras. Ignore if sent using the command protocol.</param>

The only other way I can see of managing this is to use some kind of camera manager, in the same way as gimbal manager. Using that approach the gimbal manager could re-route a command to a gimbal with the correct component ID.

Let's see what @Davidsastresas and @julianoes say - Julian is overseas so might take a while to reply.

[rmackay9]

Hi @hamishwillee, thanks for the detailed feedback.

• I think handling sets of cameras is a step too far that this point at least. The same thing can be accomplished with multiple mavlink or mission commands.
• If mav-cmd-image-start-capture is sent as a command to a mavlink enabled camera then I agree it should ignore the ID and just always process it. I think this is true whether the message is sent directly (e.g. the sysid/compid are set to specifically address the camera) or sent as a broadcast (e.g. sysid=0, compid=0)
• In the case of a mission (executed on an autopilot) I think the autopilot should send the command directly to the mavlink enabled camera with the sysid/compid set. The camera would ignore the "ID" column as mentioned above.

BTW, let's recognise that most cameras are not mavlink enabled. Most are much simpler and just triggered via a PWM signal. In these most common cases, the autopilot is acting like the "camera manager" and this is when the new ID field is particularly useful.

[hamishwillee]

Hi @rmackay9

I'm on other work today, so fly by ...

Yes to all.

In the case of a mission the thing that is executing the mission (autopilot) should take the ID from the camera mission item and generate an appropriately addressed camera command if using a MAVLink camera.

Do we need to think about the multi-camera-attached to flight controller case? I.e. if the autopilot sees 1-6 (say) in the id parameter it implicitly maps these to autopilot-attached cameras?

BTW, let's recognise that most cameras are not mavlink enabled. Most are much simpler and just triggered via a PWM signal. > In these most common cases, the autopilot is acting like the "camera manager" and this is when the new ID field is particularly useful.

Yes and no. I have never seen a mavlink enabled camera, but most of the setups I see are companion computer centric, and the camera is abstracted by a camera manager. We need to support both cases. Of course you know that :-)

[rmackay9]

@hamishwillee,

thanks again.

Do we need to think about the multi-camera-attached to flight controller case? I.e. if the autopilot sees 1-6 (say) in the id parameter it implicitly maps these to autopilot-attached cameras?

Yes, in ArduPilot at least, the user has to set parameters to specify the type of each camera attached. So AP has an "instance" number for each camera so we will just do a mapping from this parameter value to the instance number.

[hamishwillee]

@rmackay9 Just to be clear, is that "instance number" something we can just label as 1-6 in exactly the same way as gimbals. So I'm thinking.

<param index="1" label="target_component" minValue="0" maxValue="255" increment="1">Target camera component ID when sent in missions. 1-6: camera instance ID for autopilot-connected cameras. 0: all cameras. Ignore if sent using the command protocol.</param>

[rmackay9]

@hamishwillee,

yes, that's what I'm thinking as well. It's really the same issue but affecting cameras instead of gimbals.

[Davidsastresas]

If I understood correctly, you guys are proposing leaving that field 1-6 for camera id when the camera is managed by Autopilot, because the rest of the time the command will make it to the camera because it will have reported using its component ID, so the commands will arrive to it directly, correct? In the case of missions, the autopilot will be responsible to re route this command ( autopilot should have then a parameter or something to assign camera instances to compid or something like that ).

I like that approach. And that field, with 1-6 values, can even be useful in the case of mavlink enabled cameras with several sensors. I am thinking of a camera where you have a visible and multispectral sensor for instance. If that camera is mavlink enabled, it could understand which sensor needs to be triggered that way.

Until now, for mavlink cameras, that is managed by the extended parameter protocol. With parameters we tell the camera which sensor or group of sensors we want to trigger when we send that command. This is how Workswell cameras work for instance ( a good example of fully compliant mavlink camera ). With this approach we can also specify if that command should trigger all cameras at once. So in short, I don't think this will be an issue ever for mavlink cameras, unless we are programming a mission where we want some times to trigger one sensor and other times just other sensor, which is very unlikely I think.

About the possibility to mean sensor ID for mavlink cameras, maybe it is more involved, maybe for that we would need something like what was proposed in #1749, or maybe at some point we should rework camera protocol similar to how gimbal manager v2 work. At this point I think it would be overkill, and I think we need to see over time how this gimbal manager v2 plays with the existing systems. I am sure it will evolve. Maybe at some point gimbal manager and camera manager can be "merged", in the end of the day most use cases will have the camera attached to a gimbal.

So, in short, I am good with this change for now. I don't think it breaks anything, it solves a problem, and I don't think it makes sense to make something much more elaborated at this point. I am seeing lately a lot of movement around gimbal/camera manufacturers, so I am sure all of this will evolve a lot in the near future.

Thanks for keeping me updated!

[hamishwillee]

If I understood correctly, you guys are proposing leaving that field 1-6 for camera id when the camera is managed by Autopilot, because the rest of the time the command will make it to the camera because it will have reported using its component ID, so the commands will arrive to it directly, correct? In the case of missions, the autopilot will be responsible to re route this command ( autopilot should have then a parameter or something to assign camera instances to compid or something like that ).

@Davidsastresas Yes.

Note that I was being too mission-centric by making statements about the field like "not used in command protocol.". If an autopilot can have have up to six attached cameras the command protocol needs to be able to select those.

<param index="1" label="id" minValue="0" maxValue="255" increment="1">
Target camera ID (camera component id, 1 to 6 for cameras that don't have a distinct component id such as autopilot-attached cameras). 0: all cameras. 
This is used in missions to target specific cameras. 
This can be used in commands to target autopilot-connected cameras (1 to 6 or 0): other values should be ignored.</param></param>

It is desirable to be able to address individual sensors on a MAVLink camera, but this is a problem with just one field - because in a mission we need this to mean "MAVLink address of the target". There is no way to say in missions "this camera and this sensor".

I worry that if we enable addressing sensors on mavlink cameras for commands someone will insist we work out how to do it for missions too - and I don't think we can do this without an extra field. But if we want this then we could word it like:

<param index="1" label="id" minValue="0" maxValue="255" increment="1">Target camera/sensor ID.
This is used in missions to specify a target camera for the mission item: a MAVLink camera component ID, 1 to 6 for autopilot-attached cameras, 0: for all cameras.
For MAVLink cameras the autopilot should resend the item as a MAVLink command, setting the target_component to this value, and param1 to 0).
When sent in a command the address of the camera component is specified in target_component. This field allows you to specify a particular autopilot-connected camera (1-6) or all cameras (0). More generally, you can use it to address a particular camera sensor on a MAVLink camera that has more than one camera sensor.</param>

All a bit frustrating.

[hamishwillee]

@Davidsastresas PS Do you know any cameras other than Workswell that implement the protocol?

[Davidsastresas]

@Davidsastresas PS Do you know any cameras other than Workswell that implement the protocol?

I think PhaseOne is also implementing the protocol. I don't think I've worked directly with any other camera that uses it, so I am not 100% sure of any other.

About the missions matter, I really think for missions it could be handled in the Autopilot and/or the mavlink camera, using parameters for the main camera, whatever. I really think at this point it would be overkill to elaborate a camera manager. I mean, it will be a good idea at some point, but I feel we need to test more the idea with gimbal manager before extending it further as it is, don't you think?

[hamishwillee]

@Davidsastresas @rmackay9

Thanks. I have updated the description and param1 for one mission above and merged. It is essentially the same thing we already discussed but "spelled out". The particularly important edge case is that if the commands are broadcast, then they should not set param1 as anything other than 0 (not that I think you would want to).

If you are OK with this I would roll them into the other MAV_CMD. I would still like @julianoes to comment. Not sure if we are ready to merge after that, but you should have enough to verify/prototype.

About the missions matter, I really think for missions it could be handled in the Autopilot and/or the mavlink camera, using parameters for the main camera, whatever. I really think at this point it would be overkill to elaborate a camera manager. I mean, it will be a good idea at some point, but I feel we need to test more the idea with gimbal manager before extending it further as it is, don't you think?

Yes. I would much prefer to solve this without having to invent camera manager concepts. But I would prefer to be sure that we don't preclude them.

The cost of not addressing this now is that we don't know what cameras area directly attached to the autopilot.

Thank you for the camera information. I will add to the camera microservice docs.

[julianoes]

I think this makes sense, thanks for working it all out!

[hamishwillee]

Thanks @julianoes .

OK, rolled out the changes. I'm happy with this but the rules say we prefer to keep things unmerged until prototyped. OK for us to do that and ping us once you're sure it works for the various cases?

[nexton-winjeel]

This change will work, but don't you have to apply it to every camera message? Not just MAV_CMD_IMAGE_START_CAPTURE and MAV_CMD_IMAGE_STOP_CAPTURE?

(I got here from ArduPilot/ardupilot#24955)

[rmackay9]

@nexton-winjeel,

Yes, we will need to go through as many as possible of the camera related messages and hopefully add the field to them as well. Let's not let perfection be the enemy of the good though or we will just get stuck.

[hamishwillee]

This change will work, but don't you have to apply it to every camera message? Not just MAV_CMD_IMAGE_START_CAPTURE and MAV_CMD_IMAGE_STOP_CAPTURE?

Yes, we will need to go through as many as possible of the camera related messages and hopefully add the field to them as well. Let's not let perfection be the enemy of the good though or we will just get stuck.

FWIW this is the cost of deciding flight stacks don't have to map hardware connected components to be "real" mavlink components AND requiring that we support having more than one camera physically attached to a flight stack.
If we allowed only one connected camera, or required that all cameras have a mavlink presence, we would not need this.

The only other alternative would be to design a system that completely abstracts component ids as the addressing mechanism and use some kind of camera manager. That's a big redesign which I do not believe there is interest or support in.

Given that choice, this is the only logical approach I can see.

It would be nice to add this to the other commands, so at least we know which ones might be problematic. Perhaps once we're sure that this approach will work, via prototyping.

[hamishwillee]

PS We discussed briefly in MAV call last night. We're still happy with this. We don't require all the other messages be updated to merge this. We'll merge once there is a tested prototype (the rest of the team were happy to merge now, but I'm annoying).

[rmackay9]

We've implemented this in AP now so I think this can go in now. ArduPilot/ardupilot#24772