Mavlink: Implemented SET_CAMERA_ZOOM #13681
Conversation
| } | ||
|
|
||
| if ((int)(cmd_mavlink.param1 + 0.5f) == 2) { | ||
| actuator_controls.control[4] = cmd_mavlink.param2 / 50.0f - 1.0f; |
There was a problem hiding this comment.
Is it documented somewhere that channel 5 is zoom?
There was a problem hiding this comment.
No, channel 5 is still reserved.
There was a problem hiding this comment.
Hm, could you add it to the description of the channel then in the message?
There was a problem hiding this comment.
I added a PR to Devguide. Is that what you expected?
There was a problem hiding this comment.
And I would add a note or index in the msg file:
https://github.com/PX4/Firmware/blob/master/msg/actuator_controls.msg
Some cameras allow to control the zoom with PWM. So it makes sense to add the zoom to the actuator controls/outputs. A first implementation for handling mavlinks SET_CAMERA_ZOOM can be found in PX4/PX4-Autopilot#13681
dayjaby
force-pushed
the
pr-set-camera-zoom
branch
2 times, most recently
from
7e6a21f to
a2d8834
Compare
dayjaby
force-pushed
the
pr-set-camera-zoom
branch
from
a2d8834 to
bfd4ffb
Compare
|
msg/actuator_controls.msg
Outdated
| uint8 GROUP_INDEX_GIMBAL = 2 | ||
| uint8 GROUP_INDEX_MANUAL_PASSTHROUGH = 3 | ||
| uint8 GROUP_INDEX_MC_VIRTUAL = 4 | ||
| uint8 GROUP_INDEX_FW_VIRTUAL = 5 |
There was a problem hiding this comment.
@sfuhrer are these correct?
There was a problem hiding this comment.
Looking at the guide (https://dev.px4.io/master/en/concept/mixing.html) or here: yes, ordering is correct. But not sure where we would need the GROUP_INDEX of MC_VIRTUAL and FW_VIRTUAL atm.
There was a problem hiding this comment.
@julianoes @dayjaby What needs to happen to progress this? I'm just working through my docs PRs and this is a dependency.
| @@ -9,8 +9,17 @@ uint8 INDEX_FLAPS = 4 | |||
| uint8 INDEX_SPOILERS = 5 | |||
| uint8 INDEX_AIRBRAKES = 6 | |||
| uint8 INDEX_LANDING_GEAR = 7 | |||
| uint8 INDEX_GIMBAL_SHUTTER = 3 | |||
| uint8 INDEX_CAMERA_ZOOM = 4 | |||
There was a problem hiding this comment.
So these indexes are for another group, right? Otherwise they conflict.
|
@dayjaby can you make sure this is up-to-date and the CI passes? |
dayjaby
force-pushed
the
pr-set-camera-zoom
branch
2 times, most recently
from
e8b7aa6 to
ff86188
Compare
dayjaby
force-pushed
the
pr-set-camera-zoom
branch
from
ff86188 to
f65cadb
Compare
|
@julianoes Rebased and removed the questionable GROUP_INDEX_MC_VIRTUAL and GROUP_INDEX_FW_VIRTUAL. |
Some cameras allow to control the zoom with PWM. So it makes sense to add the zoom to the actuator controls/outputs. A first implementation for handling mavlinks SET_CAMERA_ZOOM can be found in PX4/PX4-Autopilot#13681
Describe problem solved by this pull request
Some cameras, like the FLIR Duo Pro R, don't allow to use the MAV_CMD_SET_CAMERA_ZOOM message to control the zoom. Instead they rely on PWM values.
Describe your solution
In mavlink_receiver.cpp we react on MAV_CMD_SET_CAMERA_ZOOM if CAMERA_ZOOM_TYPE is ZOOM_TYPE_RANGE and publish the zoom control in index 4 of actuator_controls_2. This allows maximum flexibility in assigning an AUX channel. Index 4 in control group 2 is still reserved, but seems to be a good spot to place the zoom (index 3 is camera shutter).
Describe possible alternatives
One alternative would be to have a parameter similar to MNT_MAN_ROLL/PITCH/YAW where we can set the AUX channel directly. But with the mixer file approach it is possible to allow PWM scaling/reversing.
mavlink_receiver.cpp is probably not the best place to do this implementation. Maybe create an own module called "camera" where we can also handle other CAMERA_ZOOM_TYPEs?
Test data / coverage
Only in PX4/sitl_gazebo on the iris with this mixer file: https://gist.github.com/dayjaby/8e451632915cd7c3ec0934ebffca8c52 and adding
set MIXER_AUX mount_zoomto ROMFS/px4fmu_common/init.d-posix/10016_irisDoing a test on the real drone today. (Edit: works!)