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

Python 2.7 compatibility #199

Closed
wants to merge 0 commits into from
Closed

Conversation

neomanic
Copy link
Contributor

In preparation for a ROS driver for the ODrive, I've added Python 2.7 compatibility.

Tested on macOS and Ubuntu 16.04 via direct use and pip -e installs. I haven't tested building the distribution files.

Notable changes:

  • Threading API constructor can't take the daemon parameter, so all thread creation had to be expanded out.
  • TimeoutError isn't defined, but it makes for more readable code, so I defined it as an OSError subclass.
  • ModuleNotFoundError is replaced by the older ImportError.
  • Print function imported from future

@CLAassistant
Copy link

CLAassistant commented Jul 23, 2018

CLA assistant check
All committers have signed the CLA.

@neomanic
Copy link
Contributor Author

Just noticed a few issues after generating the distribution tar.gz and installing from that.

odrivetool has python3 set in the hash-bang header. I've been running it with python2 odrivetool .... Not sure of the best option here, but anyone wanting use odrivetool completely under 2.7 will need to tweak this.

I also haven't yet used the DFU functionality, but that has a print function that needs updating to import from future.

tests.py uses type annotations, which aren't backwards-compatible, obviously. I'm thinking just to leave this alone, as I'm not too concerned with actually running the tests.

@madcowswe
Copy link
Collaborator

Awesome thanks so much! It may take quite a while until we have enough time to get around to this PR, but py2 and ROS support is something we definitely want.

Note to self (and others who see this), @neomanic's ROS driver is here: https://github.com/neomanic/odrive_ros

@neomanic
Copy link
Contributor Author

neomanic commented Aug 9, 2018

I needed to have my own master branch on my fork so I can make some changes without requiring all my py2.7 compat work along for the ride, which is now on it's own branch. I'll reopen the pull request off the new branch, just ping me when you have the time to review.

@madcowswe madcowswe moved this from Future roadmap to Features recently DONE in ODrive Development Oct 22, 2018
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.

None yet

3 participants