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

Make DO_SET_SERVO work #10320

Closed
wants to merge 5 commits into from
Closed

Conversation

kpetrykin
Copy link
Contributor

@kpetrykin kpetrykin commented Aug 24, 2018

Hello!

My goal was to make the mission command DO_SET_SERVO working with the possibility of triggering the servo with RC switch.

First, I have found that "vmount" module (when enabled) publishes its values to actuator_controls_2 topic all the time. This overwrites all other publications to this topic. That is why DO_SET_SERVO is not working.

I modified "vmount" module so that it would publish only if one of the actuator values were changed by RC. In other cases it stays silent.

Second, I have found that the formula, which translates PWM value from DO_SET_SERVO command parameters to -1..1 range for uORB, does not match the one which translates it back.
Original formula: actuators.control[(int)item.params[0]] = 1.0f / 2000 * -item.params[1];
Formula, which translates value back: effective_pwm[i] = control_value * (max_pwm[i] - min_pwm[i]) / 2 + (max_pwm[i] + min_pwm[i]) / 2;
New formula: actuators.control[(int)item.params[0]] = (float)(((float)item.params[1] - (PWM_DEFAULT_MAX + PWM_DEFAULT_MIN) / 2)/((PWM_DEFAULT_MAX - PWM_DEFAULT_MIN) / 2));

I need to discuss two questions about the formula:

  • to make it work properly we need to get actual "max_pwm[i]" and "min_pwm[i]" into navigator module. I have found a way to get them, but I don't understand how to use it correctly
  • when I tested mission with DO_SET_SERVO command, servo works nice only with 1, 3 and 4 aux outputs. The second one for some reason outputs into first. I think there is something wrong with casting here: (int)item.params[0]

I have tested everything with 1.7.3 Firmvare (px4fmu-v2_default), but did not save the flight log.
I will test the last master in a few days.

@dagar
Copy link
Member

dagar commented Aug 24, 2018

There are a few problems with this PR, but let's take a step back first.

How do people want/expect DO_SET_SERVO to even work?
screen shot 2018-08-24 at 18 55 07

Which servo does it correspond to? What's supposed to happen in the not entirely unlikely scenario that that PWM channel is already used for something?

What's the real use case for DO_SET_SERVO? Would we be better implementing a couple higher level commands? payload, gripper, etc

If the answer is still DO_SET_SERVO then the right place to implement that command is in the px4io (MAIN pwm) and/or px4fmu (AUX pwm) drivers. Those drivers are the consumers of actuator_controls_{0, 1, 2, 3} and work directly with PWM values.

@LorenzMeier
Copy link
Member

@4ert We need to separate the different actuator groups for good to make this easier. Could you join the PX4 dev call this week?
https://dev.px4.io/en/contribute/#dev_call

@kpetrykin
Copy link
Contributor Author

  • How do people want/expect DO_SET_SERVO to even work?
    In our case we want to release a cargo by the mission command. The cargo is locked by the servo which is connected to one of the AUX outputs. We want to use DO_SET_SERVO command to send a PWM signal (parameter 2) to the AUX output from paramter 1.

  • Which servo does it correspond to?
    As we can see now the Navigator publish DO_SET_SERVO values to actuator_controls_2 topic. This control group is used to rule gimbal:
    image

  • What's the real use case for DO_SET_SERVO? Would we be better implementing a couple higher level commands? payload, gripper, etc
    I think when you use DO_SET_SERVO command - you just want to send an exactly PWM to external servo connected to your controller. If you want to rule gimbal - you will use DO_MOUNT_CONTROL, if you have any flight control mechanizm connected to the AUX - you will want to use some more convenient commands for this.

  • If the answer is still DO_SET_SERVO then the right place to implement that command is in the px4io (MAIN pwm) and/or px4fmu (AUX pwm) drivers. Those drivers are the consumers of actuator_controls_{0, 1, 2, 3} and work directly with PWM values.
    So, I just made a little repair into an existing WORKING mechanizm. For now, Navigator module publishes into an "actuator_controls_2" topic and the px4fmu driver gets values from it and sends to the servos. There's just two things which discourage the process: the vmount driver spamming with its own values to the topic and the formula which translates data wrong. If you switch vmount off ("vmount stop" command into a console) and correct the formula - DO_SET_SERVO will work as expected. And don't forget to load a mixer for AUX outputs.

  • What's supposed to happen in the not entirely unlikely scenario that that PWM channel is already used for something?
    I think, this question is up to the one who builds and sets up the specific device. If you connects some critical stuff to the AUX - edit mixers or just don't use DO_SET_SERVO.

@kpetrykin
Copy link
Contributor Author

kpetrykin commented Aug 27, 2018

Made a flight, the log is here: https://logs.px4.io/plot_app?log=8fb644ab-1fe4-449a-9b0f-c4656fdae029#Nav-Actuator-Outputs-AUX
The mission was:

  • takeoff to 10 meters
  • DO_SET_SERVO on AUX1 1000 PWM
  • go to waypoint
  • DO_SET_SERVO on AUX1 2000 PWM
  • land
  • DO_SET_SERVO on AUX1 1000 PWM (example releasing cargo)
  • takeoff to 10 meters
  • RTL

@kpetrykin
Copy link
Contributor Author

@LorenzMeier my speaking english is not very good but I could try to join the devcall

@kpetrykin kpetrykin changed the title Make DO_SET_SERVO working [WIP] Make DO_SET_SERVO work Aug 28, 2018
@maitham
Copy link

maitham commented Sep 5, 2018

@kpetrykin I'm currently trying to get this to work I've used your commit. So I have my Pixhawk connected to a servo and I'm trying to get the servo to move within the mavproxy shell by running servo set 1 1000. However the message I keep getting back is that the command was acknowledged but is unrecognised (result:3) . I can get the servo moving using nuttx shell, but I really want to be able to send a mavlink command to set the servo pwm is there any way of doing this?

@hamishwillee
Copy link
Contributor

@MaithaM1 It looks like the MAV_CMD_DO_SET_SERVO is set in missions and handled here:
https://github.com/PX4/Firmware/blob/master/src/modules/navigator/mission_block.cpp#L424
(FYI @bkueng this is related to mavlink/MAVSDK#533)

I can't find the message being handled outside of missions. While that does not mean it isn't, the fact that you're getting "unrecognised" is indicative.

There is quite a bit of discussion above about this. @dagar did the devcall come to any conclusions? Is there a short term recommendation on how people should approach this?

@hamishwillee
Copy link
Contributor

@dagar regarding #10320 (comment) (your questions) there are some good points there. Below are "in my opinion".

Which servo does it correspond to?

Shouldn't this be defined at the MAVLink/message level by a convention? How about 1-n are the MAIN outputs as labeled on board, and n upwards are the AUX ports, if present?

What's supposed to happen in the not entirely unlikely scenario that that PWM channel is already used for something?

IMO things screw up - it is up to the user of this tool to manage contention. See real use case below.

Do we actually manage contention otherwise? ie does the system check that we don't have multiple control group inputs assigned to the same output?

What's the real use case for DO_SET_SERVO?

The real use case is probably the hardware we haven't thought about. Someone has some magic hardware that they want to drive, and a free port. They just want to be able to plug in that hardware and send it a command via MAVLink. They don't want to care about mixers. They are happy to manage contention themselves - ie they know that nothing else is connected to the port, and if they screw up they screw up.

Would we be better implementing a couple higher level commands? payload, gripper, etc

We currently have First Payload which is just a parachute. Yes we should probably define something for a gripper. Not sure it will address the above use case.

I've read http://dev.px4.io/en/concept/mixing.html and I am not 100% confident that my understanding of how these work is complete. I think that control groups are implemented as UORB topics that something can set (ie a controller). A mixer defines what actuators are set and their values based on these inputs.

So essentially you create a control group so that you can arbitrarily map a specific type of command to a physical output. For example to use a parachute you write a handler for it in MAVLink that sets the appropriate control group UORB topic. "Something" in the architecture then takes the mapping in the mixer file and based on that topic input sets the appropriate PWM output. Is that about right?

This is good because your system does not need to know what plugs into the output to implement a parachute or gripper (say). But it does mean that you need a mixer.

Question: Are mixer's additive? Ie I have an airframe which uses the main outputs. I add a parachute. Do I need a completely new mixer file, or can I add my mixer just for my parachute?

You could implement this as a servo control group(s). But would be better if the mapping was transparent (ie no specific mixer required).

@kpetrykin
Copy link
Contributor Author

@MaithaM1 this PR is generally about mission's DO_SET_SERVO command.
It looks like if you want to pass it through mavlink it would not be handled.
You may experiment with writing your own handler somewhere here: https://github.com/PX4/Firmware/blob/039221fa93303a6840a45c3f8a72580aaaf2d74c/src/modules/mavlink/mavlink_receiver.cpp#L527
Or you may assign an rc trigger for your servo and try to override it with mavlink message, but it is not a good way.
And what do you want to do? What is your case?

@maitham
Copy link

maitham commented Sep 6, 2018

@kpetrykin Thanks for getting back to me. I want to simply change pwm output from a script.
Currently I thought this would be possible via sending the MV_CMD_DO_SET_SERVO using pymavlink,
it doesn't seem to be implemented, unless i'm missing something (which is highly likely as this is all new to me) but I'm open to other ways?

@kpetrykin
Copy link
Contributor Author

@MaithaM1 so what is your goal of using servo? What you try to do with this?
And from what you send MAV_CMD_DO_SET_SERVO using pymavlink? Onboard computer?

@maitham
Copy link

maitham commented Sep 6, 2018

@kpetrykin I would like run certain tests to change servo positions, its a static test so nothings flying. I just want the ability to set servos to specific angles and motors to specific speeds within a python script. I don't intend to use it for flying. Its just for testing.

Sending MAV_CMD_DO_SET_SERVO using pymavlink!

@kpetrykin
Copy link
Contributor Author

So, @dagar, @LorenzMeier - what's with this PR? What could i do to improve it?

@tubeme
Copy link

tubeme commented Oct 24, 2018

Hey @kpetrykin Can you share your mixer that you are using for the AUX channel? I'm trying to follow yur route but cannot make the mixer properly maybe.

@kpetrykin
Copy link
Contributor Author

@tubeme, sure, it's simple:

M: 1
O:      10000  10000      0 -10000  10000
S: 2 0  10000  10000      0 -10000  10000

M: 1
O:      10000  10000      0 -10000  10000
S: 2 1  10000  10000      0 -10000  10000

M: 1
O:      10000  10000      0 -10000  10000
S: 2 2  10000  10000      0 -10000  10000

M: 1
O:      10000  10000      0 -10000  10000
S: 2 3  10000  10000      0 -10000  10000

To make it work:

  • Place a mixer into a /etc/mixers/pass.aux.mix on your pixhawk's SD card
  • Create /etc/extras.txt with the folowing command: mixer load /dev/pwm_output1 /fs/microsd/etc/mixers/pass.aux.mix

@kpetrykin
Copy link
Contributor Author

So, @tubeme, did you get the mixer to work?

@GaoGeolone
Copy link

@kpetrykin How to make it work, bro?

@bys1123
Copy link
Contributor

bys1123 commented Dec 1, 2018

Is there any new update?

@kpetrykin
Copy link
Contributor Author

@GaoGeolone what have you done already?
@bys1123 no, it's still as it is

@GaoGeolone
Copy link

@kpetrykin I think I have already make it work, thank you~I just want to trigger my servo at a certain position. And by making a change to the AUXMIXER pass file , as well as modifying the Formula in Mission_block.cpp.

@stale
Copy link

stale bot commented Mar 4, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Admin: Wont fix label Mar 4, 2019
@JamesChooWK
Copy link

Hi @kpetrykin , I want to follow what you have done the flight for example takeoff, go to a waypoint and drop a cargo. Now, I am trying to follow and write a same mixer that you provide. But there are some questions that I want to ask:

Q1) Why do we create a "extrats.txt" file with a command line "mixer load /dev/pwm_output1 /fs/microsd/etc/mixers/pass.aux.mix" ?
Q2) Where do we place this "extrats.txt" file ? same folder as our mixer file?
Q3) How can we upload the mixer and this "extrats.txt" file into our Pixhawk's SDcard? via QGC?
Q4) If QGC is needed, do we need to place these two file into our Firmware before uploading it?

Sorry for asking these silly questions. Thanks.

@stale stale bot removed the Admin: Wont fix label Mar 6, 2019
@hamishwillee
Copy link
Contributor

@JamesChooWK The Extras.txt is documented here: https://dev.px4.io/master/en/concept/system_startup.html#customizing-the-system-startup

It is used for system startup customisations - basically you're adding the mixer at runtime rather than as part of the firmware build. This is much easier for examples, testing, and when you're doing something that only applies to your setup.

You can copy the files onto the card using whatever mechanism you like - ie File explorer on windows. The files need to be present on the SD card before booting PX4. Again, they are not part of firmware, but are loaded as though they were.

@stale stale bot added the stale label Jun 17, 2020
@hamishwillee
Copy link
Contributor

@bkueng Who would I have to kill to get an implementation of this on your job queue? Yet another query on how to do this on slack today.

@coderkalyan
Copy link
Contributor

@hamishwillee I'd be willing to take a look at this if someone could give me a couple pointers as to what needs to be done!

@coderkalyan
Copy link
Contributor

Ok, I'm going to create a new PR that just maps the servo instance number from DO_SET_SERVO to actuator_group_6. It will still require the mixer to be setup as shown in https://dev.px4.io/master/en/concept/custom_mixer_payload.html, so there isn't any bypassing mixer or related weird junk.

@hamishwillee
Copy link
Contributor

@coderkalyan I'm going to connect you do Beat. Wait before starting. Just composing my thoughts. .

@hamishwillee
Copy link
Contributor

@bkueng We have a volunteer to work on this ^^^ @coderkalyan . Can you outline the solution you propose for this - or would it be best for you guys to set up a meeting or discuss in the dev call?

The architectural issue you raised above is that MAV_CMD_DO_SET_SERVO isn't really fit for purpose.

This has two fields, and two problems:

  • servo instance number - it is not clear how these map to the physical hardware/bus - ie MAIN, AUX, UAVCAN
  • pwm value. - This only works if your system uses PWM values, and you know the range.

We can make a best effort to clarify that message better - e.g. we could state that that instance values of 1-8 map to MAIN and 9-16 map to AUX. UAVCAN not supported.

Ultimately though, we should probably create a new message. Any thoughts on how we would do that? I'd be tempted by

  • bus (enum) 0: MAIN PWM, 1 AUX PWM, 2 UAVCAN, 3 ...
  • instance number in bus (uint), indexed from1 - whatever
  • Value: A scaled range from -100 to +100% that can be indicated as "invalid". Perhaps a float with NaN indicating "ignore".

@coderkalyan
Copy link
Contributor

@hamishwillee @bkueng I agree that a new message would be far less ambiguous, and allow a more flexible method of control. Maybe something on the lines of MAV_CMD_DO_SET_ACTUATOR or such, since this applies not just to servos but to generic auxiliary actuators (anything thats not part of main flight control and is not already part of its own setup, like gimbal). Just a few considerations that I'm bringing to the table:

  1. 0: MAIN PWM

    Should control of the MAIN ports be allowed? I guess that there are use cases for that, but it seems like a very hazardous thing to do.

  2. I don't think we want to bypass the mixers/control groups. On that note, should the command be restricted to a specific hard-coded control group (like new err.h breaks px4io build #6 payload) or do we give the user more flexibility in that regard? I'm worried giving too much flexibility might leak implementation details but maybe not.

  3. Following up from the previous point, does the bus need further specification? Such as which control group, or details about what the published UAVCAN message should look like?

  4. What happens if the user wants to control 2 or more actuators at "exactly" the same timing, possibly as part of a more complex payload where 2 servos have to be synchronized? The cleanest solution I can think of to solve this problem is to have an (optional) field be a "cookie", so that the user could setup/queue multiple actuator controls in multiple messages, then trigger them all at once.

  5. I do want to keep in mind that we don't want to go outside the scope of the original intent - to make DO_SET_SERVO work. However, there are more complex payload actions that could require more messages - i.e. the queuing system mentioned above, or precise timed offsets for different actuators (this isn't just in theory, btw; my use case's payload would probably use one or more of these features). Again, I should probably just focus on implementing a single actuator control message for this PR. But if anyone has any thoughts on these more advanced features, I'd love to hear them.

Side note: I am actually quite interested in being able to use UAVCAN to control my actuators. It didn't come to mind before @hamishwillee brought it up, but in my use case, even with a PWM value (which is currently mapped to a mixer connected to an RC controller) I have another microcontroller on the receiving end which is reading the PWM values and deciding what to do, so UAVCAN seems like a good choice. I haven't used UAVCAN before though, so any clarification/pointers on how this would work are appreciated!

@hamishwillee
Copy link
Contributor

@coderkalyan I'm keeping quite until @bkueng has a chance to talk. My 2 bits though:

  1. I hadn't thought what it would be called, but MAV_CMD_DO_SET_ACTUATOR sounds reasonable.
  2. My preference is that we allow any output to be touched in the message, but the system reject a command that sets an output that already has any assigned mixer outputs. Don't know if that is possible.

@bkueng
Copy link
Member

bkueng commented Jun 25, 2020

Awesome, thanks @coderkalyan. I imagine something like this:

  • a new mavlink command, MAV_CMD_DO_SET_ACTUATOR_AUX:
    • param1: aux1 [-1,1], nan=ignore
    • param2: aux2 [-1,1], nan=ignore
    • ...
  • it specifically controls the AUX channels in Control Group #3. So it goes through the mixer, and thus can be used for UAVCAN as well for example. And by using group 3, we simplify the setup, as most mixers already have it configured, so most users can just use it.
    This means it's handled together with RC, and needs deconfliction. I imagine as a parameter:
    • auto: use the last changing input: this allows to send a mavlink command, but to override later via RC and vice versa.
    • RC only: ignore mavlink commands
    • mavlink only: ignore RC

You're limited to 3 channels, but I think that is enough for the majority of the use-cases. Also I think if you need more advanced logic, you're better off adding some code to handle that specifically.

Let me know if you have questions, you can also ping me on slack directly.

@coderkalyan
Copy link
Contributor

Thanks for the clarification @bkueng! Seems reasonable, I'll get to work.

@hamishwillee
Copy link
Contributor

Hi @coderkalyan ,
Just for my interest, how is this coming along now you have the new MAV_CMD_DO_SET_ACTUATOR message?

@coderkalyan
Copy link
Contributor

@hamishwillee It was brought to my attention that my PR for DO_SET_SERVO was in conflict with another open PR that @dayjaby is working on. However I haven't had a chance to look at what exactly is conflicting and sort that out yet.

@hamishwillee
Copy link
Contributor

Thanks. I hope you can resolve this with @dayjaby and move forward!

@stale
Copy link

stale bot commented Dec 25, 2020

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

@stale stale bot added the stale label Dec 25, 2020
@LorenzMeier
Copy link
Member

Thanks for your earlier contribution. Unfortunately as the project has overall evolved quite a bit, this change doesn't apply any more. Closing stale pull requests like this one is part of us working aggressively to bring down the PR review time so that we will be able to merge or reject PRs in the future in a much more timely fashion.

@coderkalyan
Copy link
Contributor

FYI @kpetrykin #16758 should cover the same intention as this PR.

@DronecodeBot
Copy link

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/using-do-set-actuator-to-releasing-the-payload/28686/3

@ahmedfarouk99
Copy link

@kpetrykin @hamishwillee @bkueng
here is my setup:
1-i'm using f450 generic quad-rotor with pixhawk-4 and px4 firmware
2-i'm using aux pwm instead of main to decrease letancy according to px4 documentation
3- i connected my 4 motors to first 4 pwm aux pins
4- i changed parameter to use actuator tab instead of motor tab and i assigned the 4 motors to pwm aux (1-4)
5- i tested my f450 and everthing works great
6- i need to use an external servo, so i assigned it to pwm aux pin-5 and 5v external battery
7- i added servo-1 in the actuator tab as pwm aux-5
8- once i added this servo , it appears in the scroller tab next to my 4 motors used ,and i'm able to manually control the 4 motors and the servo through the scroller to check if they are working good, and all of them were working good
9- i needed to assign servo-1 used to rc channel 5 , so i modified " rc_map_aux5=channel 5" ,but when i used rc channel 5 , nothing happened....
can anyone help me please...
thanks in advance

@ahmedfarouk99
Copy link

@kpetrykin @bkueng @hamishwillee ?

@hamishwillee
Copy link
Contributor

hamishwillee commented Jul 4, 2024

@ahmedfarouk99 YOu have necrobumped an old closed issue. I don't look at those. If you have a technical support issue, ask on the support forums. If you have a bug, raise your own bug report. You should try MAV_CMD_DO_SET_ACTUATOR instead though, it works.

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

Successfully merging this pull request may close these issues.

None yet