-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Bidirectional Control For Motors #10778
base: master
Are you sure you want to change the base?
Bidirectional Control For Motors #10778
Conversation
- All Motor slider will only command motors which are not configured as bidirectional - Still need to look at implementing a more cleaner solution for the slider qml
removing debugging code squash
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, but this requires a bit more work. The metadata should fully define what you need.
Then for the slider it would be nice if there was a dead-zone in the center, that snaps and sends the default value (stops motors) if set. Similar as for the lower part for standard motors.
src/Vehicle/Actuators/Actuators.cc
Outdated
min_value = -1.0f; | ||
default_value = 0.0f; | ||
isBidirectional = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is that if reversible
is set (https://github.com/mavlink/mavlink/blob/master/component_metadata/actuators.schema.json#L37), then the range [-max, -min]
should be used for the reversible part. So you don't have to hardcode values.
src/Vehicle/Actuators/Actuators.cc
Outdated
float default_value = actuatorType.values.defaultVal; | ||
|
||
if(isMotor){ | ||
QString bidirectional_param("CA_R_REV"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot use hardcoded parameter values here, as it makes it PX4-specific, and it will break easily.
Rather we need to set the reversible
function on the PX4 side for the parameter, and then make use of it here. -> https://github.com/mavlink/mavlink/blob/master/component_metadata/actuators.schema.json#L63
Any updates on this? |
Not too much currently, will see if I can get something useful over the weekend. |
So far I have implemented the snapping for the bidirectional motor, however I a bit lost to how the metadata gets through from PX4 to QGC. So any reference to a doc or even a existing example would be great help! |
It's through the COMPONENT METADATA protocol. On PX4 you find the file under |
@bkueng Do we need to add a section on actuator metadata in docs - i.e. at least here? https://docs.px4.io/main/en/advanced/px4_metadata.html#px4-metadata-translation-publishing-params-events |
Yes I think that would be good. I can create a draft. |
Hi Beat So the current issue I am having is that the parameter CA_R_REV determines whether the slider for testing the actuator is set to 0 to 1 or -1 to 1. To not use the CA_R_REV parameter directly, the metadata scheme needs to provide a keyword or info to allow us to fetch the parameter. (this is what I am assuming) If we keep the metadata as is, the CA_R_REV parameter will live within the ActuatorType object, but this class doesn't provide info about the semantic of the parameter (function), so I will either need to search for the label of the parameter (in this case: "Bidirectional") which ends up being the same as just searching for the parameter. (Which we dont want to do) What I think I should do is: I actually have implemented the above, but just in a different branch. I will see if I can merge those changes into this branch and push. The other option is allow the parameter to live within the MixerParameter struct, thus just adding the "Reversible" term to the Function enum class (thus changing where CA_R_REV is defined within the yaml file), allowing us to extend the Function class, but then we need to alot of more searching I hope this makes more sense, and even more when I push the changes. |
- update slider to accomodate NaN for default values on bidirectional motors
Cool, nice work! I'll need to go through the details, but I think you're on the right track.
That makes sense. |
This PR implements actuator testing for bidirectional motors with the focus on gaining feedback on its implementation. This PR has only been tested with UAVCAN motors with the following PX4 PR: PX4/PX4-Autopilot#21656.
The following features are included:
There might be better ways to implement this feature:
@bkueng
bidirectional_control.mp4