junzis / pyModeS

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

Duplicate (possibly) code in bds05.py:altitude function #91

Closed wrobell closed 3 years ago

wrobell commented 3 years ago

In the bds05.py:altitude function

    if tc < 19:
        altcode = altbin[0:6] + "0" + altbin[6:]
    else:
        altcode = altbin[0:6] + "0" + altbin[6:]

Both lines are the same, so... duplicate or a bug?

wrobell commented 3 years ago

This is the changeset, which is causing above confusion

     if tc < 19:
-        # barometric altitude
-        q = mb[15]
-        if q:
-            n = common.bin2int(mb[8:15] + mb[16:20])
-            alt = n * 25 - 1000
-        else:
-            alt = None
+        altcode = altbin[0:6] + "0" + altbin[6:]
     else:
-        # GNSS altitude, meters -> feet
-        alt = common.bin2int(mb[8:20]) * 3.28084
+        altcode = altbin[0:6] + "0" + altbin[6:]
+

IMHO, this is wrong change as ADS-B messages with typecode >= 20 are treated as barometric ones now.

junzis commented 3 years ago

You are right @wrobell. Thanks for spotting this. It has been fixed in the lastest code now. I am not sure how did that happen :man_facepalming: :smiley:

wrobell commented 3 years ago

Would adding more unit tests help?

I have some specific data (i.e. barometric/qbit and gnss) and specific altitude calculation unit tests at: https://gitlab.com/wrobell/vmodes/-/blob/master/vmodes/tests/test_parser_altitude.py. Please feel free to take data from there. I also hope to bring more there soon.