petebachant / ACSpy

Python package for working with ACS motion controllers.
MIT License
23 stars 21 forks source link

encoded all strings passed to C library utf_8 #9

Closed hstarmans closed 8 years ago

hstarmans commented 8 years ago

If a string is passed to C library it's encoded to UTF8. Unicode created problems with Python 3. "string".encode(encoding='utf_8', errors='strict')

petebachant commented 8 years ago

Thanks for the PR! Have you tried running the tests with these modifications? Better yet, can you write a test that fails with Python 3 before this commit and passes after? When I run tests with

nosetests -v 

on Python 3, I get a failure for the acsc.writeReal function. Is that fixed with your implementation?

Note: This may be fixing #7 and #8.

hstarmans commented 8 years ago

This fixes 7 and 8. With the right encoding, i am able to write and read variables to /from the controller. I havent performed the tests. Just wanted to provide a quick fix as it doesnt work at the moment. I will looj at tests, maybe next week; still too busy.

Op 19 feb. 2016 om 18:00 heeft Pete Bachant notifications@github.com het volgende geschreven:

Thanks for the PR! Have you tried running the tests with these modifications? Better yet, can you write a test that fails with Python 3 before this commit and passes after? When I run tests with

nosetests -v on Python 3, I get a failure for the acsc.writeReal function. Is that fixed with your implementation?

Note: This may be fixing #7 and #8.

— Reply to this email directly or view it on GitHub.

petebachant commented 8 years ago

I just ran the tests on Python 2.7 and 3.5 and everything looks good. Is it necessary to provide those arguments to encode? The tests appear to pass just fine without them.

hstarmans commented 8 years ago

it could be these are the default settings. Only utf8 is most likely needed, strict was suggested by my integrated developer environment.

Op 19 feb. 2016 om 21:04 heeft Pete Bachant notifications@github.com het volgende geschreven:

I just ran the tests on Python 2.7 and 3.5 and everything looks good. Is it necessary to provide those arguments to encode? The tests appear to pass just fine without them.

— Reply to this email directly or view it on GitHub.

petebachant commented 8 years ago

Looks like those are the default arguments anyway: https://docs.python.org/3/library/stdtypes.html#str.encode

I'm going to merge this in with no args for the sake of cleanliness. Thanks again!