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

basic config: add actuators page (replacing motors) #1704

Merged
merged 6 commits into from Jan 12, 2022
Merged

Conversation

bkueng
Copy link
Member

@bkueng bkueng commented Dec 15, 2021

Initial docs for dynamic control allocation, containing tips and conventions on how to use the new UI (mavlink/qgroundcontrol#9952).

  • I'll probably add a direct link from QGC to this page (will need a metadata extension)
  • We could add airframe-type specific sections, if there's more specific things to be explained
  • Adding more screenshots for different types might be useful, but I did not want to add too many already as they're still in flux

A trim value can be configured for control surfaces, which is also applied to the test slider.
:::

Note the following behaviour:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we perhaps have at least some of this stuff first as safety warnings before the guidance? e.g. kill switch, safety button, and perhaps removing props?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we could. Although the switch in the UI already says it.

@hamishwillee
Copy link
Collaborator

@bkueng Good start. Some questions in line which will help me nitpick a bit more.

  1. Given this is not the default I suggest "for now" we keep both this and the motors page (rather than replacing). On the motors page add a note that this is being superseded - with link.
  2. There are questions inline about what airframe selection actually configures. I tried to test but my VM is screwed. Reinstalling.
  3. Yes, this will need more images for other airframes, but as you say, might be too early.
  4. A video would be great, but could wait too.
  5. Might be too early, but are there any other sections in config that will need to change because of this. Specifically I am thinking things like RC / actuator passthrough setup and in http://docs.px4.io/master/en/config/radio.html#additional-radio-setup

My main concern is that it isn't quite clear how much of this you have to do for what airframe. It's a bit like the tuning stuff - we used to say you can just fly a matching airframe without "having" to do this. Now it sounds like we might be having fewer airframes and we expect people to do this.

@bkueng
Copy link
Member Author

bkueng commented Dec 16, 2021

My main concern is that it isn't quite clear how much of this you have to do for what airframe. It's a bit like the tuning stuff - we used to say you can just fly a matching airframe without "having" to do this. Now it sounds like we might be having fewer airframes and we expect people to do this.

Assigning the outputs will be the main (and only) change for the user, if it's not an exotic one. I still forsee that we have generic airframes for different MC types (i.e. quad, hexa, coax, ...), where the geometry is preconfigured appropriately.

en/config/actuators.md Outdated Show resolved Hide resolved
en/config/actuators.md Outdated Show resolved Hide resolved
en/config/actuators.md Outdated Show resolved Hide resolved

The sliders snap into place at the lower end, and motors are turned off (disarmed).
The "minimum thrust" position is the next slider position, which commands the minimum thrust.
Adjust the minimum output value such that the motors spin at that slider position.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we do this? On this screen we can configure max/min value for some servos but not for motors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can mention that this is mostly relevant when PWM is used. For DShot it's not required.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add that, but can you confirm that, for PWM, this is something you have to do outside of this screen/configuration? i wonder if we should provide PWM range setting controls for motors using PWM?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, min/max will show up if you have PWM selected (as in the screenshot), and only be hidden if DShot is selected.

![Tilt Axis](../../assets/config/actuators/tilt_axis.png)


### Bidirectional Motors
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bkueng Note, I have updated this section from reversible to bidirectional motors. Why? Because we were also using reversible to mean a motor that can configure to go in either direction (but not switch while in use)

Do you think perhaps we could also change QGC to use the term "bidirectional"?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bkueng Did you see this? Are you otherwise good for this to merge?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes bidirectional is fine.

  • instead of 'required', can you use something like 'you have the option to...'? Even for rovers you don't need it, if you don't care about only going forwards.
  • the option will only be displayed if relevant for the frame type -> you always have the option under 'advanced'

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes bidirectional is fine.

It would be even better if you changed it in QGC :-).

I've added as (accepted) suggestions below. Note that when I pressed advanced checkbox in Sim last time I did not see the reversible option, which is why I assumed this is a feature of the airframe. Nevertheless I'll assume the sim is incorrect, not the intent.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already in the pipeline: PX4/PX4-Autopilot@d23b5fd :-)
Hmm did you check on the geometry side?

image

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is simulated iris from 23 December, with advanced checked.

image

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in todays build:
image
So we can forget this

@hamishwillee
Copy link
Collaborator

There are some comments above, but I think this is probably OK to go in now. We can update again when FW and VTOL are fully supported or when control allocation is fully enabled.

Let me know if you want me to merge or not.

@hamishwillee
Copy link
Collaborator

@bkueng This is good initial docs for now. Thanks. Merging.

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.

None yet

3 participants