search

junzis / pyModeS

Python decoder for Mode S and ADS-B signals
GNU General Public License v3.0
527 stars 151 forks source link

Make `callsign` function accept messages as bytes. #67

Closed wrobell closed 4 years ago

wrobell commented 4 years ago

The pyModeS accepts messages in hex string format only. The change allows the pyModeS API to accept messages in binary format also.

Once all functions are changed to accept both message formats, the internals of pyModeS can be flipped to use binary data. This, hopefully, will make the library bit faster for applications processing binary messages only.

See also discussion in https://github.com/junzis/pyModeS/issues/61.

xoolive commented 4 years ago

The thing with your implementation is that it breaks the basic test:

pms.adsb.callsign(b"8D406B902015A678D4D220AA4BDA")

(The compiled version works based on this assumption, even though the tests do not reflect this)

Maybe we need to distinguish both bytes representations.

wrobell commented 4 years ago

Well, if we want to support real binary data, then hex string as bytes type will be hard to distinguish from its real binary representation.

I will just repeat my argument from #61 - it is more efficient to store binary version of ADS-B messages. Having that, conversion by pyModeS binary -> hex string -> binary again makes the library inefficient.

Also, I can see that pyModeS/extra/rtlreader.py converts data to hex string, which is causing above conversion issue as well.

xoolive commented 4 years ago

I understand your point, it sounds valid. I am not saying it should be ignored.

There is performance, poor choices made in the past, and also backward compatibility to keep in mind. Maybe @singledispatch is not the best way to deal with this issue (maybe it is), or maybe it is a bit premature to have this dispatch in the library only for callsign?

I'll leave it to @junzis, he is the boss here, but I think we should keep your comment in mind when refactoring the whole set of core functions for v3.

junzis commented 4 years ago

I used str for the compiled version too. Basically all functions use str input type, bytes will not work.

I am not saying this is the best option, but it has the best compatibility. Hey guys, pyModeS was from Python 2 time. Probably there are still some Python 2 pyModeS users out there. :)

However, I completely agree with @xoolive. Such change has to be done once for all, preferably in a major update later. We also need to be able to support both str and bytes, in both Python and Cython implementations. So a lot of work there.

So, for the moment, I prefer to wait and give it some more thoughts from my side. I will keep you guys updated.

wrobell commented 4 years ago

No worries, just wanted to propose something constructive in the context of #61.

wrobell commented 4 years ago

Regarding Python 2, it is worth considering https://pythonclock.org/ and https://python3statement.org/, I believe.