-
Notifications
You must be signed in to change notification settings - Fork 29
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
ping device simulation #33
Conversation
866df1f
to
96f4bed
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Just a few details.
@Williangalvani done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, missing executable permission.
|
@patrickelectric use python3 |
The permission is the same as everything else. |
@jaxxzer I'm using. |
@jaxxzer Same problem with a fresh arch installation. |
@patrickelectric This requires bluerobotics/ping-protocol#118 merge |
Ok, this should be tagged in the PR. |
I forgot. Now it is. |
|
brping/ping1d-simulation.py
Outdated
@@ -0,0 +1,165 @@ | |||
#!/usr/bin/python -u |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should use env and not python directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python3 only
#!/usr/bin/env python3
or
Python2 only
#!/usr/bin/env python2
or
Both python versions
#!/usr/bin/env python
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shebang should use /usr/bin/env and not python directly
This matches the case in the rest of the repository. |
done |
No description provided.