Tigge / antfs-cli

Extracts FIT files from ANT-FS based sport watches such as Garmin Forerunner 60, 405CX, 310XT, 610 and 910XT.
MIT License
312 stars 76 forks source link

Add python3 support #112

Closed Toilal closed 9 years ago

Toilal commented 9 years ago

This adds python 3 support, but also makes minor changes

This requires the python3 branch of openant (https://github.com/Tigge/openant/pull/4)

Tested using antfs-cli on python 2.7.8 and python 3.4.1 with the Garmin Forerunner 610

mgr01 commented 9 years ago

Hi!

I've tested downloading and uploading on Python 2.7.8 with both master and python3 branches of openant -- works fine (except for unrelated #104 issue and a bunch of timeouts).

I've also tested this on Python 3.4.2 (with python3 branch of openant of course) -- found just a few minor errors and I've pointed them out in the comments on the diffs.

The only problem that I have is that antfs-cli.py hangs every time at the sys.exit() with message:

libusbx: warning [libusb_close] internal signalling write failed, closing anyway

Everything before that is fine... and it only happens on python 3. This could be related to walac/pyusb#66 but I haven't looked into it (yet).

Anyway: great job @Toilal :)

Toilal commented 9 years ago

Thanks for reviewing so fast. I'll fix small issues you have mentionned in comment tomorrow. I have the same issue with pyusb on python 3. Le 28 déc. 2014 21:42, "Mateusz Greszta" notifications@github.com a écrit :

Hi!

I've tested downloading and uploading on Python 2.7.8 with both master and python3 branches of openant -- works fine (except for unrelated #104 https://github.com/Tigge/Garmin-Forerunner-610-Extractor/pull/104 issue and a bunch of timeouts).

I've also tested this on Python 3.4.2 (with python3 branch of openant of course) -- found just a few minor errors and I've pointed them out in the comments on the diffs.

The only problem that I have is that antfs-cli.py hangs every time at the sys.exit() with message:

libusbx: warning [libusb_close] internal signalling write failed, closing anyway

Everything before that is fine... and it only happens on python 3. This could be related to walac/pyusb#66 https://github.com/walac/pyusb/issues/66 but I haven't looked into it (yet).

Anyway: great job @Toilal https://github.com/Toilal :)

— Reply to this email directly or view it on GitHub https://github.com/Tigge/Garmin-Forerunner-610-Extractor/pull/112#issuecomment-68218754 .

Toilal commented 9 years ago

@mgr01 I've fixed both PR with the comments you have made. For the PyUSB issue, it's definitively related to https://github.com/walac/pyusb/issues/66 and PR https://github.com/walac/pyusb/pull/78 fix this.

mgr01 commented 9 years ago

You are right -- PyUSB's PR fixes this issue for me. I've found 4 more minor issues. Take a look at them when you have a moment -- sorry for being so picky :).

Toilal commented 9 years ago

No problem @mgr01, it's just great to find a github projects where maintainers are available to review PR quickly. In some projects sometimes weeks are required :)

I've fixed the issues and push --force again.

mgr01 commented 9 years ago

It works great. I've retested downloading and uploading with the Garmin Forerunner 310XT on Python 2.7.8 and Python 3.4.2 with python3 branch of openant (should also work with master branch on Python 2.7 -- based on the first test) and walac/pyusb#78 version of PyUSB.

Toilal commented 9 years ago

PyUSB issue as been fixed in master.

https://github.com/walac/pyusb/issues/66#event-213300428

Tigge commented 9 years ago

@Toilal Thanks for this patch. Added a few comments, but looks good in general!

@mgr01 Thanks for reviewing and testing!

Toilal commented 9 years ago

I fixed issues you've mentionned @Tigge. But i currently experience an issue with both python version :

Traceback (most recent call last):
  File "/home/toilal/.pyenv/versions/3.4.1/lib/python3.4/threading.py", line 920, in _bootstrap_inner
    self.run()
  File "/home/toilal/.pyenv/versions/3.4.1/lib/python3.4/threading.py", line 868, in run
    self._target(*self._args, **self._kwargs)
  File "/home/toilal/garmin/openant/ant/fs/manager.py", line 136, in _worker
    self._node.start()
  File "/home/toilal/garmin/openant/ant/easy/node.py", line 128, in start
    self._main()
  File "/home/toilal/garmin/openant/ant/easy/node.py", line 121, in _main
    self.channels[channel].on_burst_data(data)
  File "/home/toilal/garmin/openant/ant/fs/manager.py", line 170, in _on_data
    self._on_command(data[8:])
  File "/home/toilal/garmin/openant/ant/fs/manager.py", line 162, in _on_command
    c = ant.fs.command.parse(data)
  File "/home/toilal/garmin/openant/ant/fs/command.py", line 368, in parse
    return command_class._parse(data)
  File "/home/toilal/garmin/openant/ant/fs/command.py", line 91, in _parse
    return cls(*args[2:])
TypeError: __init__() missing 2 required positional arguments: 'data' and 'crc'

Problem seems to be in [ant.fs.Command._parse](https://github.com/Tigge/openant/blob/master/ant/fs/command.py#L83-L88 method) method. data value is valid (it is the same value in split branch), but _parse_args returns only 5 items instead of 7, data and crc are not parsed.

Toilal commented 9 years ago

Ok I find out why. It's because of the formatting i've made.

@classmethod
def _parse_args(cls, data):
    return struct.unpack("<BBBxIII", data[0:16])
    + (data[16:-8],) + struct.unpack("<6xH", data[-8:])

This returns struct.unpack("<BBBxIII", data[0:16]) only ...

@classmethod
def _parse_args(cls, data):
    return struct.unpack("<BBBxIII", data[0:16])\
           + (data[16:-8],) + struct.unpack("<6xH", data[-8:])

fix the issue

Toilal commented 9 years ago

All seems OK now ! Thanks @Tigge & @mgr01 for reviewing this PR. I'll try to make others soon :)

Tigge commented 9 years ago

Merged. Thanks!