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

New vehicle type: Airship #14862

Merged
merged 34 commits into from
Aug 10, 2020
Merged

New vehicle type: Airship #14862

merged 34 commits into from
Aug 10, 2020

Conversation

dan-leo
Copy link
Contributor

@dan-leo dan-leo commented May 7, 2020

As mentioned in #14792 and https://discuss.px4.io/t/new-vehicle-type-airship/16514, our team has developed a new vehicle type for the PX4 developer community. Feel free to test using the cloudship branch (based off v1.10.1) or master branch in https://github.com/flycloudline/Firmware and https://github.com/flycloudline/sitl_gazebo_airship.

# Run the following to simulate in Gazebo
make px4_sitl gazebo_cloudship

image

Use QGroundcontrol and a joystick controller to test. A virtual joystick can also be used.

@dagar
Copy link
Member

dagar commented May 7, 2020

Is LTA common terminology? Feel free to use something more descriptive like airship or blimp. It doesn't necessarily need to be a short initialism (like MC or FW).

@dan-leo
Copy link
Contributor Author

dan-leo commented May 9, 2020

LTA has been renamed to airship.

When working with airship-specific parameters in future, we do need an acronym naming convention to fit within the 16 byte limit. A few suggestions could be A_, AS_, LTA_, BL_, ZP_, DG_. What do you think?

Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

Really cool!

ROMFS/px4fmu_common/init.d-posix/2507_cloudship Outdated Show resolved Hide resolved
ROMFS/px4fmu_common/init.d-posix/2507_cloudship Outdated Show resolved Hide resolved
ROMFS/px4fmu_common/init.d-posix/2507_cloudship Outdated Show resolved Hide resolved
param set RTL_DESCEND_ALT 10
param set RTL_LAND_DELAY 1

param set PWM_MAX 2000
Copy link
Member

Choose a reason for hiding this comment

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

These are the default already, so no need to have them here

Copy link
Member

Choose a reason for hiding this comment

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

@dan-leo why did you mark this as resolved? I don't see this being addressed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bkueng I didn't realize that when defined as a pwm parameter they already have those exact defaults. Updated, thanks!

src/modules/airship_att_control/airship_att_control.hpp Outdated Show resolved Hide resolved
@imnobodyet
Copy link

@dan-leo great work! is this support 6DOF type? I'm working on a project using a balloon with 6 motors mounted in the middle which can make it move directly up/down/laterally, there's no pitch and roll. I can manually control it by using a simple mixer, but don't know how to make it run auto mission.

set MIXER cloudship

# Configure this as airship.
# set MAV_TYPE 7
Copy link
Member

Choose a reason for hiding this comment

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

why is this commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is currently using MAV_TYPE 2. It would be ideal to use MAV_TYPE 7, but the implementation seems non-trivial at the moment.

Copy link
Member

Choose a reason for hiding this comment

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

I think it should be set to MAV_TYPE 7 and may be specify n=0 at https://github.com/PX4/Firmware/blob/master/src/modules/simulator/simulator_mavlink.cpp#L104. This way you are explicitly stating that the control inputs range from -1 ~ 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, this helps.

@dan-leo
Copy link
Contributor Author

dan-leo commented Jun 1, 2020

@dan-leo great work! is this support 6DOF type? I'm working on a project using a balloon with 6 motors mounted in the middle which can make it move directly up/down/laterally, there's no pitch and roll. I can manually control it by using a simple mixer, but don't know how to make it run auto mission.

Currently it supports yaw and thrust angle/pitch. Roll can be supported through differential thrust in the mixer setup, but is not recommended unless using a fixed thruster setup as you are using! We are hoping to include differential yaw/roll depending on the thrust angle for the specific set-up we have.

@dan-leo
Copy link
Contributor Author

dan-leo commented Jun 1, 2020

@dagar @bkueng @Jaeyoung-Lim @MaEtUgR what would you still like to be done, or what are are the next steps once both repos are complete? Thanks for your input thus far!

param set RTL_DESCEND_ALT 10
param set RTL_LAND_DELAY 1

param set PWM_MAX 2000
Copy link
Member

Choose a reason for hiding this comment

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

@dan-leo why did you mark this as resolved? I don't see this being addressed.

perf_counter_t _loop_perf; /**< loop performance counter */

static constexpr const float initial_update_rate_hz = 250.f; /**< loop update rate used for initialization */
float _loop_update_rate_hz{initial_update_rate_hz}; /**< current rate-controller loop update rate in [Hz] */
Copy link
Member

Choose a reason for hiding this comment

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

These are unused as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I removed them for now. They'll be useful in future updates!

- airship_att_control
- params in rc.airship_defaults
@bkueng
Copy link
Member

bkueng commented Jun 5, 2020

This contains a few submodule updates (NuttX, ecl and mavlink). I believe these are not on purpose, can you revert that?
Otherwise this looks good.

@antonerasm
Copy link
Contributor

I added, fixed and refactored a couple things.
Bottom line: this PR only implements full manual control (where the stick commands are directly passed to the actuators) along with some arming checks.
I could've probably handled it with the mixer only, but the yaw control is passed to the rudder motor and therefore I needed to add the arming check to avoid it spinning accidentally when unarmed (because servos are allowed to move then).
In future PRs, the controllers will be added.

@dan-leo
Copy link
Contributor Author

dan-leo commented Jun 28, 2020

Thanks @antonerasm, this is looking really good.

  • Is it worth adding a check for MAV_TYPE_AIRSHIP around line 3333 in src/modules/mavlink/mavlink_messages.cpp?
  • How about feeding thrust into the thrust_body_array around line 1357 in src/modules/mavlink/mavlink_messages.cpp?

Awesome!

@TSC21
Copy link
Member

TSC21 commented Jul 19, 2020

@dan-leo your airship model was brought in in #15377.

@dan-leo
Copy link
Contributor Author

dan-leo commented Jul 19, 2020

@TSC21, thanks.

@hamishwillee
Copy link
Contributor

When this goes in would be good to get some docs. Specifically:

  1. Simulation - in https://dev.px4.io/master/en/simulation/gazebo_vehicles.html and https://dev.px4.io/master/en/simulation/gazebo.html#running-the-simulation
  2. Anything you think is needed for someone to emulate or modify your work. That might be information about mixer setup, or a page on an airframe build: https://docs.px4.io/master/en/airframes/

Jaeyoung-Lim
Jaeyoung-Lim previously approved these changes Aug 6, 2020
Copy link
Member

@Jaeyoung-Lim Jaeyoung-Lim left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution.

It seems like all the modified parts or not on the critical path, so good for me

bkueng
bkueng previously approved these changes Aug 6, 2020
Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

One detail, otherwise good to merge.

Co-authored-by: Beat Küng <beat-kueng@gmx.net>
@antonerasm antonerasm dismissed stale reviews from bkueng and Jaeyoung-Lim via fcaa8b3 August 7, 2020 20:34
@bkueng bkueng merged commit fa4818e into PX4:master Aug 10, 2020
@dan-leo
Copy link
Contributor Author

dan-leo commented Aug 11, 2020

Yay!

@hamishwillee
Copy link
Contributor

Awesome. Docs reminder: #14862 (comment) "an undocumented feature might as well not exist"

@Jaeyoung-Lim
Copy link
Member

Jaeyoung-Lim commented Aug 14, 2020

@dan-leo Could you please follow up and add this to the docs?

@antonerasm
Copy link
Contributor

@Jaeyoung-Lim @hamishwillee Yes, I will do that ASAP. It is next on my to-do list.

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

8 participants