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

Offboard example in Python3 #74

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

TheLegendaryJedi
Copy link

To build I needed to add a new folder with a empty __init__.py

TheLegendaryJedi pushed a commit to TheLegendaryJedi/PX4-user_guide that referenced this pull request Mar 25, 2021
This should only be included after this PR is merged:
PX4/px4_ros_com#74
@TSC21
@zeroos
Copy link

zeroos commented Apr 13, 2021

Cool work, thanks, it made my life much simpler as I did not have to convert the C++ example to Python!

Some suggestions:

  • would be cool to show a way to make some fields of TrajectorySetpoint ignored, e.g., msg.x = float("NaN"). It took me a couple of hours to figure this thing out. Alternatively, would be cool if None could also be interpreted as NaN or the default value is NaN.
  • I'm not sure the comment in the documentation about changing execution rights for the script is needed, they seem to be set correctly in git.
  • This commit is a bit confusing to me, seems unrelated to this PR: f954785

If there is anything I could do to help with this PR, please let me know, I'd be happy to help!

@TheLegendaryJedi
Copy link
Author

Cool work, thanks, it made my life much simpler as I did not have to convert the C++ example to Python!

Some suggestions:

  • would be cool to show a way to make some fields of TrajectorySetpoint ignored, e.g., msg.x = float("NaN"). It took me a couple of hours to figure this thing out. Alternatively, would be cool if None could also be interpreted as NaN or the default value is NaN.
  • I'm not sure the comment in the documentation about changing execution rights for the script is needed, they seem to be set correctly in git.
  • This commit is a bit confusing to me, seems unrelated to this PR: f954785

If there is anything I could do to help with this PR, please let me know, I'd be happy to help!

Thanks a lot for the contributions!

Yes, that commit is nothing related to this PR.

If you want, after merged, you can edit it and add those comments regarding the "NaN" and submit a new PR.

Thanks!

@hamishwillee
Copy link
Contributor

Any progress on this? I ask because of the associated docs PR :-)

@TheLegendaryJedi
Copy link
Author

TheLegendaryJedi commented Jun 25, 2021

Any progress on this? I ask because of the associated docs PR :-)

I don't know if it's good to merge (just the 09b5a07). @TSC21 can you review it?

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.

3 participants