PropGit
commented
1 year ago Fantastic find @ThiloA! You're right!
Did you find this by trying to understand the code, or by <_gulp_> being unable to download certain P1 code?
Line 38's "3-BIT" element should be: /%00111/ {0xF5, 3}
Specifically, to encode 3 bits whose value happens to be %111 into the start of the next byte to serially transmit; the byte value to transmit must be 0xF5. That's %11110101. That is transmitted LSB first, and with the start bit (0) prepended and the stop bit (1) appended), that's these ten bits: %1111101010. The P1 translates single low bits (including start bit) as a binary 1 and double low bits (including start bit) as a binary 0. Thus, this translation (remember LSB first; i.e. three single 0s) is %111. The mistaken value in the existing code would cause it to "translate" to %10 (wrong bit size and very wrong value).
At first I couldn't believe it because that code has been in use for 4.5 years - how could it be that it hasn't caused a noticeable problem for so long? It might be that it's very rare for the P1 binary to end with "3" final bits to encode and that those bits just happen to be %111. Is that 1 chance out of 240? Nope. It's much less likely than that, and difficult to calculate the chances since the specific bit pattern of the leading stream is itself encoded in chunks of 3, 4, or 5 bits at a time (depending on pattern) and the stream size (in bytes) varies as well.
I checked, and sure enough, that value was faithfully copied from my original code (where I also hadn't detected my mistake; in code checking, nor through testing).
I've fix it here now, via PR #54 and commit a1b4cd8
The 3 Bit encoding in serialloader.cpp line 38:
/*%00111*/ {0xFA, 3}seems to be wrong. It is the same encoding as used for 2 Bits (line 33):
/*%00010*/ {0xFA, 2}Manual encoding reveals:
0xFA => StopBit(1) 11111010 StartBit(0) => 1111110100 => only 2 bits are encoded: (10)