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

moved set_camera_zoom and do_set_servo to commander #14809

Closed
wants to merge 2 commits into from

Conversation

dayjaby
Copy link
Contributor

@dayjaby dayjaby commented May 2, 2020

Describe problem solved by this pull request
SET_CAMERA_ZOOM was handled in mavlink_receiver before and DO_SET_SERVO only in mission_blocks. Moved both to the Commander.cpp. Also changed the formula for DO_SET_SERVO, as it probably required some very specific mixer before. Now it merely scales the 0-2000 input in the command to -1 to 1 and the rest is up to the mixer.

@julianoes
Copy link
Contributor

I'm a bit confused here. What exactly does it fix? And why?

@dayjaby
Copy link
Contributor Author

dayjaby commented May 5, 2020

I'm a bit confused here. What exactly does it fix? And why?

Both SET_CAMERA_ZOOM and DO_SET_SERVO can be mission items now and be performed if you send a COMMAND_LONG.

Before this PR, you could have DO_SET_SERVO as mission item, but not as command, and SET_CAMERA_ZOOM as command, but not as mission item.

@julianoes
Copy link
Contributor

Right. And this actually works? 😲

@dayjaby
Copy link
Contributor Author

dayjaby commented May 5, 2020

Right. And this actually works?

After reaching waypoint with SET_CAMERA_ZOOM, param1=2, param2=50:

~/Development/FirmwarePX4/build/px4_sitl_default/bin$ sudo ./px4-listener actuator_controls_2

TOPIC: actuator_controls_2
 actuator_controls_s
	timestamp: 414596000  (19.948000 seconds ago)
	timestamp_sample: 0
	control: [nan, nan, nan, nan, 0.0000, nan, nan, nan]

After reaching waypoint with SET_CAMERA_ZOOM, param1=2, param2=100:

~/Development/FirmwarePX4/build/px4_sitl_default/bin$ sudo ./px4-listener actuator_controls_2

TOPIC: actuator_controls_2
 actuator_controls_s
	timestamp: 436496000  (9.784000 seconds ago)
	timestamp_sample: 0
	control: [nan, nan, nan, nan, 1.0000, nan, nan, nan]

After reaching waypoint with DO_SET_SERVO, param1=1, param2=1500:

~/Development/FirmwarePX4/build/px4_sitl_default/bin$ sudo ./px4-listener actuator_controls_2

TOPIC: actuator_controls_2
 actuator_controls_s
	timestamp: 458816000  (18.656000 seconds ago)
	timestamp_sample: 0
	control: [nan, 0.5000, nan, nan, nan, nan, nan, nan]

A manual DO_SET_SERVO:

~/Development/FirmwarePX4/build/px4_sitl_default/bin$ rosservice call /mavros/cmd/command "{broadcast: false, command: 183, confirmation: 0, param1: 1, param2: 2000, param3: 0.0,
  param4: 0.0, param5: 0.0, param6: 0.0, param7: 0.0}" 
success: True
result: 0
~/Development/FirmwarePX4/build/px4_sitl_default/bin$ sudo ./px4-listener actuator_controls_2

TOPIC: actuator_controls_2
 actuator_controls_s
	timestamp: 539864000  (1.084000 seconds ago)
	timestamp_sample: 0
	control: [nan, 1.0000, nan, nan, nan, nan, nan, nan]

A manual SET_CAMERA_ZOOM:

~/Development/FirmwarePX4$ rosservice call /mavros/cmd/command "{broadcast: false, command: 531, confirmation: 0, param1: 2, param2: 100, param3: 0.0,
  param4: 0.0, param5: 0.0, param6: 0.0, param7: 0.0}" 
success: True
result: 0
simadm@10:~/Development/FirmwarePX4$ sudo ./build/px4_sitl_default/bin/px4-listener vehicle_command

TOPIC: vehicle_command
 vehicle_command_s
	timestamp: 847064000  (14.288000 seconds ago)
	param5: 0.000000
	param6: 0.000000
	param1: 2.0000
	param2: 100.0000
	param3: 0.0000
	param4: 0.0000
	param7: 0.0000
	command: 541
	target_system: 1
	target_component: 1
	source_system: 1
	source_component: 240
	confirmation: 0
	from_external: True
simadm@10:~/Development/FirmwarePX4$ sudo ./build/px4_sitl_default/bin/px4-listener actuator_controls_2

TOPIC: actuator_controls_2
 actuator_controls_s
	timestamp: 859448000  (5.292000 seconds ago)
	timestamp_sample: 0
	control: [nan, nan, nan, nan, 1.0000, nan, nan, nan]

@julianoes julianoes requested review from bkueng and dagar and removed request for bkueng May 6, 2020 06:25
Copy link
Contributor

@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 this is good. I just would like @dagar to double check it.

@dayjaby dayjaby force-pushed the pr-commander_set_zoom_and_servo branch from 4c822b3 to 6c0e026 Compare May 6, 2020 08:09
@dayjaby
Copy link
Contributor Author

dayjaby commented Jun 23, 2020

@hamishwillee I think you are more on track on the DO_SET_SERVO requests on slack...is this a feasible solution for it? The only problem is the collision with actuator_controls_2 from the vmount module.

@bkueng
Copy link
Member

bkueng commented Jun 24, 2020

I prefer to avoid the collision, and rather go for aux in control group #3 as outlined in #10320 (comment) with a new command. That way most existing setups can just use it w/o mixer changes and it is testable via RC as well.
Zoom should go through vmount. How are you using that currently, without vmount?

@dayjaby
Copy link
Contributor Author

dayjaby commented Jun 24, 2020

I prefer to avoid the collision, and rather go for aux in control group #3 as outlined in #10320 (comment) with a new command. That way most existing setups can just use it w/o mixer changes and it is testable via RC as well.
Zoom should go through vmount. How are you using that currently, without vmount?

The camera that we use is happy with getting the PWM signal just once for setting the zoom, so publishing actuator_controls_2 once was sufficient to get the zoom controlled, of course with a corresponding mixer file.

@coderkalyan
Copy link
Contributor

It was just brought to my attention that my PR for DO_SET_SERVO (#15281) conflicts with this. Not really sure what's going on here, or who should continue DO_SET_SERVO work? After talking with @bkueng, I am currently working on mapping DO_SET_SERVO to control group 3 RC aux 1-3. I am also adding support for the new MAV_CMD_DO_SET_ACTUATOR message.

@stale
Copy link

stale bot commented Oct 12, 2020

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants