Hello,
the driver kyd_IP620.py has a channel memory structure of 16 bytes but printing it shows a length of 15 bytes. I get this output:
struct { memory: struct { rx_freq: 4:[(0)] tx_freq: 4:[(0)] rx_tone: 0x0000 tx_tone: 0x0000 unknown_1: 0x00 (....0000b) busy_loc: 0x00 (......00b) n_a: 0x00 (......00b) unknown_2: 0x00 (.......0b) scan_add: 0x00 (.......0b) w_n: 0x00 (.......0b) lout: 0x00 (.......0b) n_a_: 0x00 (.......0b) power: 0x00 (......00b) unknown_3: 0x00 unknown_4: 0x00 } memory (15 bytes at 0x0000)
} (anonymous) (15 bytes at 0x0000)
using the following script === from chirp import bitwise
defn =""" struct { // Channel memory structure lbcd rx_freq[4]; // RX frequency lbcd tx_freq[4]; // TX frequency ul16 rx_tone; // RX tone ul16 tx_tone; // TX tone u8 unknown_1:4, // n-a busy_loc:2, // NO-00, Crrier wave-01, SM-10 n_a:2; // n-a u8 unknown_2:1, // n-a scan_add:1, // Scan add n_a:1, // n-a w_n:1, // Narrow-0 Wide-1 lout:1, // LOCKOUT OFF-0 ON-1 n_a_:1, // n-a power:2; // Power low-00 middle-01 high-10 u8 unknown_3; // n-a u8 unknown_4; // n-a } memory[1]; """
data = bytearray(b"\x00" * 16) tree = bitwise.parse(defn, data) print(tree) ===
I do not own such a radio but the same issue could be present in other drivers. I think that the reason is that n_a is used twice, so I reduced the definition to this
defn =""" struct { u8 n_a:2; u8 n_a:1; } memory[1]; """
but this throws a RecursionError exception at .../chirp/bitwise.py", line 878, in do_bitfield LOG.warn("WARNING: %i trailing bits unaccounted for in %s" %
I would submit a pull request but I don't know how to handle the following issues in bitwise.py: * line 878 doesn't correctly handle the name that it needs to print, but I don't know how to fix it * the __repr__ of class structDataElement and possibly 4 other lines of code shouldn't use the integer division self.size() // 8 or the size should be automatically padded to the next multiple of 8 with a dummy member? * should there be a warning for reused names of struct members or an exception?
thanks -- 73 de IU5HKX Daniele
I do not own such a radio but the same issue could be present in other drivers. I think that the reason is that n_a is used twice
Yep, I think you're right. I've hit this problem before myself while writing a driver, but it usually bites me in a way that causes me to adjust the driver and I always think "I better make bitwise check that" but I never do.
So I wrote up this quickly:
https://github.com/kk7ds/chirp/pull/907
I was going to make it reject them, which is the right behavior, but a lot of drivers are committing this sin, and some drivers in many many places. So, I made it an ERROR log and auto rename so we can proceed. I also added a test and marked it as XFAIL so we can see which drivers are broken. Here's the list currently:
[gw3] [ 2%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Retevis_RB17P::test_bitwise_errors [gw7] [ 5%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Wouxun_KG-UVD1P::test_bitwise_errors [gw9] [ 7%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Leixen_VV-898S::test_bitwise_errors [gw8] [ 12%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Baofeng_UV-5R::test_bitwise_errors [gw7] [ 12%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Radtel_RT-470L::test_bitwise_errors [gw7] [ 15%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Baofeng_UV-9G::test_bitwise_errors [gw5] [ 15%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Radtel_RT-470X::test_bitwise_errors [gw9] [ 18%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Leixen_VV-898::test_bitwise_errors [gw8] [ 20%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Baofeng_F-11::test_bitwise_errors [gw1] [ 27%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Yaesu_FT-2900R_1900R::test_bitwise_errors [gw0] [ 28%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Jetstream_JT270MH_A_Band::test_bitwise_errors [gw9] [ 29%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Wouxun_KG-UV6::test_bitwise_errors [gw6] [ 32%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Radioddity_UV-5G::test_bitwise_errors [gw5] [ 33%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Yaesu_FT-70D::test_bitwise_errors [gw0] [ 36%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Jetstream_JT270MH_B_Band::test_bitwise_errors [gw6] [ 43%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Explorer_QRZ-1::test_bitwise_errors [gw6] [ 49%] XFAIL tests/test_drivers.py::TestBitwiseStrict_BTECH_MURS-V2::test_bitwise_errors [gw5] [ 50%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Radtel_RT-470::test_bitwise_errors [gw9] [ 52%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Yaesu_FT-7100M_VHF_Band::test_bitwise_errors [gw8] [ 55%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Vertex_Standard_FTL-2011::test_bitwise_errors [gw6] [ 58%] XFAIL tests/test_drivers.py::TestBitwiseStrict_BTECH_GMRS-V2::test_bitwise_errors [gw3] [ 59%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Yaesu_FT-450::test_bitwise_errors [gw4] [ 60%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Yaesu_FT-450D::test_bitwise_errors [gw5] [ 62%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Retevis_RA89::test_bitwise_errors [gw9] [ 67%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Yaesu_FT-7100M_UHF_Band::test_bitwise_errors [gw6] [ 70%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Abbree_AR-730::test_bitwise_errors [gw3] [ 71%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Baofeng_UV-6R::test_bitwise_errors [gw3] [ 76%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Hiroyasu_HI-8811::test_bitwise_errors [gw9] [ 82%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Baofeng_UV-S9X3::test_bitwise_errors [gw6] [ 82%] XFAIL tests/test_drivers.py::TestBitwiseStrict_TYT_TH-UV88::test_bitwise_errors [gw5] [ 84%] XFAIL tests/test_drivers.py::TestBitwiseStrict_TYT_TH-UV8000::test_bitwise_errors [gw3] [ 85%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Jetstream_JT270M::test_bitwise_errors [gw7] [ 88%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Puxing_PX-888K::test_bitwise_errors [gw2] [ 89%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Baofeng_BF-A58S::test_bitwise_errors [gw7] [ 90%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Baofeng_5RX::test_bitwise_errors [gw1] [ 91%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Anysecu_UV-A37::test_bitwise_errors [gw4] [ 92%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Talkpod_A36plus::test_bitwise_errors [gw3] [ 93%] XFAIL tests/test_drivers.py::TestBitwiseStrict_KYD_IP-620::test_bitwise_errors [gw8] [ 93%] XFAIL tests/test_drivers.py::TestBitwiseStrict_Icom_IC-V80::test_bitwise_errors [gw9] [100%] XFAIL tests/test_drivers.py::TestBitwiseStrict_AnyTone_5888UVIII::test_bitwise_errors
So please, everyone, if you care about a driver on that list, help by fixing the name clashes. Just loading an image should give you some logs. The leixen driver (which accounts for multiple entries above) is a particularly bad offender. Here's what it looks like when loading with that patch applied:
ERROR: Duplicate definition for unknown on line 10: unknown:2,; renaming to unknown_000186 ERROR: Duplicate definition for unknown on line 12: unknown:1;; renaming to unknown_000186 ERROR: Duplicate definition for unknown1 on line 28: unknown1:1,; renaming to unknown1_000189 ERROR: Duplicate definition for unknown2 on line 31: unknown2:2;; renaming to unknown2_000189 ERROR: Duplicate definition for unknown on line 41: u8 unknown:3,; renaming to unknown_000190 ERROR: Duplicate definition for unknown on line 52: unknown:2,; renaming to unknown_000195 ERROR: Duplicate definition for unknown on line 54: u8 unknown:4,; renaming to unknown_000196 ERROR: Duplicate definition for unknown1 on line 58: u8 unknown1:3,; renaming to unknown1_000197 ERROR: Duplicate definition for unknown2 on line 60: unknown2:1,; renaming to unknown2_000197 ERROR: Duplicate definition for unknown1 on line 64: u8 unknown1:3,; renaming to unknown1_000199 ERROR: Duplicate definition for unknown1 on line 66: u8 unknown1:3,; renaming to unknown1_00019a ERROR: Duplicate definition for unknown1 on line 68: u8 unknown1:2,; renaming to unknown1_00019b ERROR: Duplicate definition for unknown1 on line 70: u8 unknown1:1,; renaming to unknown1_00019c ERROR: Duplicate definition for unknown1 on line 72: u8 unknown1:2,; renaming to unknown1_00019d ERROR: Duplicate definition for unknown1 on line 74: u8 unknown1:3,; renaming to unknown1_00019e ERROR: Duplicate definition for unknown on line 76: unknown:4;; renaming to unknown_00019e
The fix is just to rename the dupes to something else. Hopefully the actual number of drivers is smaller than the lines above (like leixen) and fixing a few files will make a big dent. Note the line number above is the line number from the start of the memory format text, not the line in the python file.
I'll plan to merge that PR before the next build.
defn =""" struct { u8 n_a:2; u8 n_a:1; } memory[1]; """
but this throws a RecursionError exception at .../chirp/bitwise.py", line 878, in do_bitfield LOG.warn("WARNING: %i trailing bits unaccounted for in %s" %
This is complaining because you defined a u8 and only defined two bits of it in the first case. The second one only has one bit. The fix is to avoid the name clash between the two "n_a" variables. I'm guessing neither are used, and thus just renaming them to something else will work.
The recursion error is because pyPEG has some bugs in serializing its output.
- the __repr__ of class structDataElement and possibly 4 other lines
of code shouldn't use the integer division self.size() // 8 or the size should be automatically padded to the next multiple of 8 with a dummy member?
Nothing but a bitfield element can be aligned to less than 8 bits, so I think it's okay as it is (modulo the bug).
- should there be a warning for reused names of struct members or an exception?
Absolutely. Log now, exception eventually.
Thanks Daniele!
--Dan
So please, everyone, if you care about a driver on that list, help by fixing the name clashes.
Just FYI, I've got fixes for the leixen and icv80 drivers queued up. The icv80 was doing a similar thing to what Daniele reported, which was set_raw(15) on a memory object that was supposed to be 16 bytes long. Probably because the author tried 16 and bitwise complained, thinking it was only 15 bytes long and just changed it to appease. That means that driver hasn't been clearing memories entirely, leaving the last byte untouched! So, definitely a good thing to get fixed, especially in the popular drivers...
--Dan
participants (2)
-
Dan Smith
-
Daniele Forsi