search

Tigge / openant

ANT and ANT-FS Python Library
MIT License
184 stars 85 forks source link

Pad erase request and add erase helper function #3

Closed jonnylamb closed 9 years ago

jonnylamb commented 9 years ago

The first patch probably isn't good, but it works. I'm not sure how you want padding done in openant but the erase request needs padding. Feel free to just implement it properly and drop this patch or just tell me how you want it.

The second patch is less obviously wrong.

Tigge commented 9 years ago

Thank for this merge request! Looks good -- a few pointers though:

The preferred way to do padding in this instance is to use some additional 'x' in the _format string (which is passed to struct.unpack/pack. See for example the DisconnectCommand class here which also does this. The AuthenticateBase class use another method since the 'x' method doesn't work in that case (variable data).

The second patch (erase in Application should probably follow the flow of download and upload and throw a AntFSEraseException containing the error code.

Tigge commented 9 years ago

I took the liberty of fixing the issues and merged this branch in 4e4415cc30268b3c9447003a42e37e88d2fd0b3b.