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

[PX4IO/PWM driver] Added trim values to the PWM output drivers #5488

Closed
wants to merge 2 commits into from

Conversation

bartosz-wawrzacz
Copy link
Contributor

This adds a way to specify the trim/center position of the output PWM signal.
Makes the setup of control surfaces on fixed wing aircraft much easier.
It might look like it's doubling the mixer trim/center functionality, but it's much easier to have a generic mixer for a given airplane type and set the exact servo throws for each unit separately.
Tested, flown and used by the fixed wing group of ETH ASL.

@kd0aij
Copy link
Contributor

kd0aij commented Sep 13, 2016

How does one use this capability?
Also, it doesn't look like the trim values are saved in non-volatile memory; is that correct?

@bartosz-wawrzacz
Copy link
Contributor Author

Sorry for not clarifying the usage...
This adds a trim option to the pwm command and has the same syntax as the pwm min or pwm max command.
So for example:
pwm trim -c 1 -p 1600 sets the center position for PWM channel 1 to 1600.

And you are correct, this is not saved in the non-volatiole memory, so has to be put into the start script on the SD-card. The pwm min and pwm max commands are also 'temporary', so I didn`t change the whole thing.

@bartosz-wawrzacz
Copy link
Contributor Author

Also, looks like there's some mixer test that I wasn't aware of, as we only use the Pixhawk board and it doesn't seem to be used there. I will adapt that soon.

@kd0aij
Copy link
Contributor

kd0aij commented Sep 13, 2016

Thanks, that makes sense. You trim on the bench using the console terminal to issue "pwm trim" commands. Do you put the necessary commands into extras.txt on the SD card?
I'm not sure why this would be easier than just editing a custom mixer on the SD card though. Is it easier to edit the script than the mixer? I know there have been problems with the mixer parsing in the past.

@bartosz-wawrzacz
Copy link
Contributor Author

We are using a custom start script directly in /etc/rc.txt and add all the pwm commands there.

AFAIK to edit the mixer you have to take the SD card out, change stuff, put the card back in and reboot/reload the mixer. Trimming using the commands in the console is much less of a hassle. Naturally you have to paste the values into the start script in the end.

And as mentioned before, it's nice to have a generic mixer file for a given airplane type if you have more then one. Still, you need to set the min/center/max values for all the servos and we use the pwm command to achieve that.

Also, I should have mentioned that - if you set the trim value to 0 (which is also the default), it ignores it and works exactly as before.

@kd0aij
Copy link
Contributor

kd0aij commented Sep 13, 2016

You're right, I had forgotten how difficult it was to customize a mixer.
But with these changes, you could add a bunch of new PWM parameters to specify channel trims, and then you could trim using QGC. And that would eliminate the need for a startup script too.

@LorenzMeier
Copy link
Member

I think we should definitely go for parameters in this case. Then we can do trim sliders in QGC and everything gets really, really simple.

@bartosz-wawrzacz
Copy link
Contributor Author

Yes, definitely. Then I guess it makes sense to have the min, max and center as parameters. Anything else?
I'll rework this in the close future.

@bartosz-wawrzacz bartosz-wawrzacz force-pushed the add_pwm_trim branch 3 times, most recently from b142004 to d6bc689 Compare September 22, 2016 11:55
@bartosz-wawrzacz
Copy link
Contributor Author

I made the necessary changes in the other places, seems to be fine now.

Regarding the shift to parameters - after a deeper look I have to say this is not a straightforward thing, as right now there is basically no single app running that could monitor the param changes and update the IO/drivers. Right now I'm rather busy with our platforms, so I won't be able to work on that. I'd say to just use the current approach for now ;)

@LorenzMeier
Copy link
Member

That's not actually correct. The IO driver does this for all RC settings already. Would be a piece of cake to add.

@bartosz-wawrzacz
Copy link
Contributor Author

Oh yeah, that's true...

Is it safe to assume that the px4io driver is always running? I don't know the other platforms besides Pixhawk.

@dagar dagar mentioned this pull request Oct 16, 2016
@kd0aij
Copy link
Contributor

kd0aij commented Oct 18, 2016

Is anyone still working on this? I'll try to add the parameters if nobody else is doing it.

@bartosz-wawrzacz
Copy link
Contributor Author

Would be nice, as I'm working on the satcom/iridium stuff now.

@LorenzMeier
Copy link
Member

@bartosz-wawrzacz Have you seen this? #5633

@bartosz-wawrzacz
Copy link
Contributor Author

Yes, even posted a comment there :)

@PX4BuildBot
Copy link
Collaborator

Can one of the admins verify this patch?

@kd0aij
Copy link
Contributor

kd0aij commented Oct 19, 2016

The FMU uses parameters PWM_AUX_REVn to control servo reversing, and PX4IO uses PWM_MAIN_REVn. This would be correct for fmu-v2, but is it right for fmu-v4 or any other config without an IO coprocessor?

@kd0aij
Copy link
Contributor

kd0aij commented Oct 20, 2016

Regarding the definition of trim, and its application in method pwm_limit_calc:

  • each mixer has a mixer_scaler_s, which specifies negative scale, offset and positive scale
  • the mixer calls method scale, which calculates out = scale * input + offset

The scaler.offset is what one would modify to change the neutral position in a custom mixer, but this new trim value is an independent parameter.

@LorenzMeier @julianoes Do you agree that pwm_limit_calc should be modified to define "neutral" to be 1500 usec and offset==trim should be specified in normalized units using the existing mixer logic?

@kd0aij kd0aij mentioned this pull request Oct 21, 2016
@kd0aij
Copy link
Contributor

kd0aij commented Oct 23, 2016

@bartosz-wawrzacz I just got around to testing the console command here:

nsh> pwm min -c 1 -p 1000
nsh> pwm trim -c 1 -p 1000
WARN  [pwm] no PWM values added
ERROR [pwm] usage:
pwm arm|disarm|rate|failsafe|disarmed|min|max|test|steps|info  ...

I checked out branch add_pwm_trim directly from your fork, ran "make clean"
then built for fmu-v2. Any idea what might have gone wrong?

@bartosz-wawrzacz
Copy link
Contributor Author

Just pushed a fix. Dunno how this happened, I probably messed it up while doing git-black-magic.

@bartosz-wawrzacz
Copy link
Contributor Author

Anyway, looks like you're taking a different approach and modifying the mixer itself, so I don't know if this is of any use.

@kd0aij
Copy link
Contributor

kd0aij commented Oct 23, 2016

It's true that the trim command and register page would be redundant to the parameters, but I wasn't sure whether you would want to retain that functionality. It would save some RAM in the IO coprocessor if we removed the register page, but my initial implementation is still using it. Do you see any downside to eliminating the register page? If not, I'll see if I can modify the IOCTL to directly alter the mixer offsets, instead of filling the registers.

@LorenzMeier
Copy link
Member

The current implementation seems to make sense to me.

@bartosz-wawrzacz
Copy link
Contributor Author

Redone using parameters here:
#5720

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

Successfully merging this pull request may close these issues.

4 participants