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

add wall following cflib demo #390

Merged
merged 13 commits into from
Mar 31, 2023
Merged

add wall following cflib demo #390

merged 13 commits into from
Mar 31, 2023

Conversation

knmcguire
Copy link
Contributor

We have had the wall following app for a while in the crazyflie firmware. There used to be a python version made back when the SGBA work was done, but at one point all development was done using only the c version of the wall following algorithm.

I've now ported the c code back to python, so that we can have a cflib example of the wall following demo.

@knmcguire knmcguire marked this pull request as ready for review March 28, 2023 13:51
Copy link
Contributor

@krichardsson krichardsson left a comment

Choose a reason for hiding this comment

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

To me this is OK to merge, especially since it is a direct re-write of the C-code and you want to keep the 1 to 1 mapping.

There are room for improvements though, some general ideas would be:

  1. Use constants for numbers to describe what they might mean
  2. Use math.pi instead of 3.14
  3. Use math.radians() and math.degrees() when converting between radians and degrees
  4. Add a function for cleaning up multiranger values instead of repeating code
  5. A more liberal use of variables that describes the meaning of a calculation. There are some cases where calls to logic_is_close_to(), contain a fair amount of code. Readability would improve by first assigning the calculation to a variable with a descriptive name, and secondly using the variable when calling logic_is_close_to()

@knmcguire
Copy link
Contributor Author

Thanks for the tips! I've implemented a few already but I'll add issues to both the crazyflie-firmware and crazyflie-lib-python to implement these in the future. It would be good to keep the two versions synced up.

@knmcguire knmcguire merged commit 6370208 into master Mar 31, 2023
@knmcguire knmcguire deleted the wallfollowing branch March 31, 2023 06:45
@knmcguire knmcguire added this to the next release milestone Apr 11, 2023
@knmcguire knmcguire mentioned this pull request May 2, 2023
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