Skip to content

Adapt latest crazyflow - #139

Merged
ratheron merged 7 commits into
learnsyslab:mainfrom
Yuming-Lee24:adapt-latest-crazyflow
Sep 27, 2026
Merged

ratheron merged 7 commits into
learnsyslab:mainfrom
Yuming-Lee24:adapt-latest-crazyflow

Conversation

@Yuming-Lee24

@Yuming-Lee24 Yuming-Lee24 commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

I pinned the crazyflow to the latest commit in the pyproject.toml and reproduced the import error on my linux desktop.

commit 1:
now the param loader will be imported from the crazyflow.dynamics and takes in both dynamics model and the drone model. I also changed the signature of build_action_space() to load dynamics.

commit 2:
the params are no longer expand according the world number since learnsyslab/crazyflow#105, a few lines are added in race_core.py to enable it, solved the incompativle shapes for broadcasting error in tests

commit 3:
adapt to the 16 dim of state action. Notice that in race_core.py:

        def apply_action(action: Array, data: EnvData) -> None:
            """Apply the commanded state action to the simulation."""
            ...
            if "action" in disturbances:
                ...
                action += disturbances["action"](subkey, action.shape)
                ...
            return data.replace(sim_data=ctrl_fn(data.sim_data, action))

the disturbance will be added slightly different as before, since it adds on xyzw now.

testing result: all pass on linux and macos, passed the deployment test

@Yuming-Lee24

Copy link
Copy Markdown
Collaborator Author

the deploy.py did not work, let me check

@Yuming-Lee24
Yuming-Lee24 marked this pull request as draft September 24, 2026 10:52
@Yuming-Lee24

Copy link
Copy Markdown
Collaborator Author

everything is working now

@Yuming-Lee24
Yuming-Lee24 marked this pull request as ready for review September 24, 2026 11:24

@ratheron ratheron left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for updating to the latest Crazyflow version, which has some breaking changes.

Added some comments. We should also check the docs for the quat, since only the yaw from the quat is used in the control stack.

Comment thread lsy_drone_racing/control/state_controller.py Outdated
Comment thread lsy_drone_racing/envs/race_core.py Outdated
Comment thread lsy_drone_racing/envs/race_core.py Outdated
Comment thread lsy_drone_racing/envs/real_race_env.py Outdated
Comment thread lsy_drone_racing/envs/real_race_env.py Outdated
Comment thread tests/deploy/test_deployment.py Outdated
Comment thread tests/integration/test_controllers.py Outdated
Comment thread pyproject.toml Outdated
Comment thread lsy_drone_racing/utils/crazyflie.py Outdated
Comment thread lsy_drone_racing/control/attitude_mpc.py Outdated

@ratheron ratheron left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just one minor thing left for me to approve. Btw, don't we need to change any docs? Seems odd.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In my opinion, the parameter thrust_max is always per motor, so we shouldn't overwrite it in the dict with 4x. Instead, just take it x4 where we write it into the thrust constraints, as it was before.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yes make sense, but there is somewhere else in the code has ambiguity on total thrust_max and on thrust_max per motor, for example in the build_action_space(), I will suggest to change the variouble name to self.total_thrust_max there.

@Yuming-Lee24

Copy link
Copy Markdown
Collaborator Author

Just one minor thing left for me to approve. Btw, don't we need to change any docs? Seems odd.

you mean the pr for docs? it haven't done yet, I will finish it tomorrow. Or what do you mean?

@ratheron

Copy link
Copy Markdown
Collaborator

Just one minor thing left for me to approve. Btw, don't we need to change any docs? Seems odd.

you mean the pr for docs? it haven't done yet, I will finish it tomorrow. Or what do you mean?

I mean we change the crazyflow version which broke things. If we mention load_params somewhere in the docs, this also needs to be changed. If the docs never talk about this, it's fine

@Yuming-Lee24

Copy link
Copy Markdown
Collaborator Author

Just one minor thing left for me to approve. Btw, don't we need to change any docs? Seems odd.

you mean the pr for docs? it haven't done yet, I will finish it tomorrow. Or what do you mean?

I mean we change the crazyflow version which broke things. If we mention load_params somewhere in the docs, this also needs to be changed. If the docs never talk about this, it's fine

I will check the docs, and should I change the variable names to fix the ambiguity in

self.thrust_min = drone_params["thrust_min"] * 4
self.thrust_max = drone_params["thrust_max"] * 4
thrust_min, thrust_max = params["thrust_min"] * 4, params["thrust_max"] * 4

@ratheron

Copy link
Copy Markdown
Collaborator

Just one minor thing left for me to approve. Btw, don't we need to change any docs? Seems odd.

you mean the pr for docs? it haven't done yet, I will finish it tomorrow. Or what do you mean?

I mean we change the crazyflow version which broke things. If we mention load_params somewhere in the docs, this also needs to be changed. If the docs never talk about this, it's fine

I will check the docs, and should I change the variable names to fix the ambiguity in

self.thrust_min = drone_params["thrust_min"] * 4
self.thrust_max = drone_params["thrust_max"] * 4

thrust_min, thrust_max = params["thrust_min"] * 4, params["thrust_max"] * 4

Yeah, we can remove the ambiguity by saying sth like total_thrust_max

@ratheron
ratheron merged commit c3a54ca into learnsyslab:main Sep 27, 2026
5 checks passed
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.

2 participants