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

Document yaw arguments of cmdPosition() and cmdVelocityWorld() #258

Open
MauroPfister opened this issue Sep 24, 2020 · 3 comments
Open

Document yaw arguments of cmdPosition() and cmdVelocityWorld() #258

MauroPfister opened this issue Sep 24, 2020 · 3 comments

Comments

@MauroPfister
Copy link
Contributor

MauroPfister commented Sep 24, 2020

While trying to get familiar with the crazyswarm project, I noticed a few things that seem wrong/not updated/inexistent in the docs. I am glad to create pull requests to fix this, but I wanted to double check with @whoenig and @jpreiss first.

  • Python Scripting API

    • The yaw argument of pycrazyswarm.crazyflie.Crazyflie.cmdPosition() seems to be in degrees and not in radians.
    • The yawRate argument of pycrazyswarm.crazyflie.Crazyflie.cmdVelocityWorld() seems to be positive in clockwise direction which does not match with the coordinate system of the CF in my opinion.
  • Tutorials - Creating a new streaming setpoint mode

    • The tutorial explains how to integrate a new streaming setpoint, but it seems to miss the part that needs to be integrated in src/crazyswarm/src/crazyswarm_server.cpp. I only figured this out by comparing src/crazyflie_ros/crazyflie_driver/src/crazyflie_server.cpp with src/crazyswarm/src/crazyswarm_server.cpp but I have not a full understanding of what needs to be implemented where. In my opinion this should be explained in the docs.
  • ROS Topics and Services
    I think it could be helpful to have a list of available topics and services (+ their arguments). For example it took me some time to realize that the duration argument of /land was in nano seconds. Maybe this is obvious to someone who is more familiar with ROS though.

If desired, I can continue to update this list whenever I find something that might be wrong in the docs. Let me know what you think.

@jpreiss
Copy link
Collaborator

jpreiss commented Sep 25, 2020

You are totally right about the crazyswarm_server.cpp issue. I believe the problem is that the Crazyswarm version links directly against https://github.com/USC-ACTLab/libobjecttracker (providing the feature of tracking multiple objects with the same marker configuration), whereas the crazyflie_ros version does not. It would be nice if we didn't have to have two versions of the file, but I guess that would require some conditional compilation setup in crazyflie_ros. @whoenig I recall discussing this with you, but cant remember if it's a WONTFIX or not.

For the yaw arguments, I'm sure you are right but I would like to double check first and add code comments for the appropriate functions/structs - somehow there was wrong/missing info in the code that caused the documentation error.

Pull requests would be great! Please make separate commits for each change (or separate PRs). For small and "uncontroversial" changes in the future, feel free to create a PR directly without discussing in an issue first.

@whoenig
Copy link
Contributor

whoenig commented Sep 25, 2020

Longer-term (perhaps early next year), the goal is to merge the two repositories. In my opinion, there is very little use of crazyflie_ros and I almost always use the crazyswarm instead (even when flying a single CF). However, crazyflie_ros still has additional features that we didn't port yet and is a bit easier to debug because it does not use multithreading.

+1 for fixing the docs!

@MauroPfister
Copy link
Contributor Author

Thanks for the clarification about crazyswarm_server.cpp.

Shall I make a PR for the yaw arguments or will you fix this yourself after testing @jpreiss ? Concerning the tutorial about creating a new streaming setpoint mode: Maybe it is better if someone with a deeper knowledge of the cpp code fixes that part.

@jpreiss jpreiss changed the title Crazyswarm documentation Document yaw arguments of cmdPosition() and cmdVelocityWorld() May 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants