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

Spacecraft Support: adding modules for Space Robotics and Thruster Based platforms #23117

Draft
wants to merge 90 commits into
base: main
Choose a base branch
from

Conversation

Pedro-Roque
Copy link
Member

@Pedro-Roque Pedro-Roque commented May 9, 2024

Solved Problem

This PR addresses support on PX4 for Spacecrafts or other space systems that use Thrusters as their main actuators. These are particularly relevant for space robotics applications.

Modules for control (with appendix sc_ ) and Thruster actuation compatibility were added.

This PR aims at integrating these additions, which will soon be published and updated on the source code, into the PX4 ecosystem to support space robotics facilities.

Solution

  • Add rate control, attitude control and position control for spacecraft-type vehicles (prefixed with ac_)
  • Add Thruster Effectiveness in the control allocation module
  • Add 2 simulation and one hardware airframes using these new modules

Changelog Entry

For release notes:

Feature/Bugfix XYZ
New parameter: XYZ_Z
Documentation: Need to clarify page ... / done, read docs.px4.io/...

Alternatives

Reusing multicopter code. However, there are multiple problems associated with it:

  1. Their actuation is not the same leading to different effectiveness models
  2. Their PWM cycle is different than thruster-based platforms, which typically use solenoid-based valves
  3. Their control can be different to take advantage of the lack of gravity
  4. Their estimator is different since their model is, too

Test coverage

To-Do

Context

Currently being integrated in https://github.com/DISCOWER/PX4-Space-Systems/tree/dev-metric_control_allocator, as part of the DISCOWER project (a publication documenting this contribution as well as the entire lab facilities will be published by the end of August). Pictures and videos will be added soon. Continued from #23098

Pedro-Roque and others added 30 commits May 17, 2023 19:28
This commit adds a space systems rate control module
Update sitl_gazebo submodule hash
Add Space Systems Rate controller
@Pedro-Roque Pedro-Roque force-pushed the dev-merge_upstream branch from 2a2687d to 697bcde Compare May 13, 2024 11:51
Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

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

Only had a quick glance to see if I can spot any high level things but didn't go through everything to be honest. Style check is failing. Let's work through step by step.

@@ -68,6 +68,14 @@ if(CONFIG_MODULES_MC_RATE_CONTROL)
)
endif()

if(CONFIG_MODULES_SC_RATE_CONTROL)
px4_add_romfs_files(
rc.heli_defaults
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
rc.heli_defaults

I guess that's not used for thruster robots.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. Removed.

CMakeLists.txt Outdated
PATTERN ".svn" EXCLUDE)
else()
message(STATUS "No PX4 build files to install.")
endif()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
endif()
endif()

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@@ -38,6 +38,10 @@ px4_add_romfs_files(
18001_TF-B1

# [22000, 22999] Reserve for custom models

# [70000, 79999] Spacecraft-type platforms
70000_kth_space_robot
Copy link
Member

Choose a reason for hiding this comment

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

This should be conditionally included based on CONFIG_MODULES_SC_RATE_CONTROL no?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

sc_pos_control start

# Start Space Robot Thruster Controller
sc_thruster_controller start
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
sc_thruster_controller start
sc_thruster_controller start

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed format.

Comment on lines -55 to -59
CONFIG_MODULES_FLIGHT_MODE_MANAGER=y
CONFIG_MODULES_FW_ATT_CONTROL=y
CONFIG_MODULES_FW_AUTOTUNE_ATTITUDE_CONTROL=y
CONFIG_MODULES_FW_POS_CONTROL=y
CONFIG_MODULES_FW_RATE_CONTROL=y
Copy link
Member

Choose a reason for hiding this comment

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

It's currently not an option to remove these modules from the default build. I suggest there to be a thruster robot build similar to rover. Otherwise this will not scale further.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, will look into that.

Comment on lines +3 to +6
float32 x1 # duty cycle of thruster-pair x1 [-1;1]
float32 x2 # duty cycle of thruster-pair x2 [-1;1]
float32 y1 # duty cycle of thruster-pair y1 [-1;1]
float32 y2 # duty cycle of thruster-pair y2 [-1;1]
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

@Pedro-Roque Pedro-Roque May 14, 2024

Choose a reason for hiding this comment

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

This is related to the thruster_controller module, that then publishes motor controls that get translated to actuator outputs. The x's and y's are relative to body frame, but this should probably be removed since we don't need this to be upstream.

EDIT: I'll keep it for now. Might remove later.

Comment on lines 1 to 3
uint64 timestamp # time since system start (microseconds)

char[127] text
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually no clue why this is here - will remove. Thanks for the catch!

Comment on lines +13 to +14
float32[4] attitude # in quaternions
float32[3] angular_velocity # in rad/s^2
Copy link
Member

Choose a reason for hiding this comment

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

That's what attitude setpoint and rate setpoint are for. We shouldn't combine those for all use cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

But since the pipeline is pos_ctl sending an attitude setpoint to the att controller, how do you combine trajectories that require both? Because one of the commands gets ignored in that case... or will get jittered?

@@ -87,7 +87,9 @@ static int io_timer_handler7(int irq, void *context, void *arg);
* taken into account
*/
#if !defined(BOARD_PWM_FREQ)
#define BOARD_PWM_FREQ 1000000
// #define BOARD_PWM_FREQ 1000000
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't stay commented out. Could the downscaling be something board-specific that breaks other boards?

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 could probably make this configurable. It's mostly for thruster based systems that require a smaller frequency

@Pedro-Roque
Copy link
Member Author

Thanks Matt, I'll start pushing on this now! :)

@Pedro-Roque
Copy link
Member Author

Should sync with PX4/PX4-SITL_gazebo-classic#1039

@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/px4-sync-q-a-may-15-2024/38773/1

@@ -86,8 +86,11 @@ static int io_timer_handler7(int irq, void *context, void *arg);
* We also allow for overrides here but all timer register usage need to be
* taken into account
*/
#if !defined(BOARD_PWM_FREQ)
#if !defined(BOARD_PWM_FREQ) && !defined(CONFIG_MODULES_SC_RATE_CONTROL)
Copy link
Member

Choose a reason for hiding this comment

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

At a minimum let's do this via kconfig and not introduce module specific ifdefs.

@@ -663,7 +666,8 @@ int io_timer_init_timer(unsigned timer, io_timer_channel_mode_t mode)
* default to updating at 50Hz
*/

timer_set_rate(timer, 50);
timer_set_rate(timer, 10);
Copy link
Member Author

Choose a reason for hiding this comment

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

@Pedro-Roque big mess, change this

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.

7 participants