[andlaus]

this allows to pack and unpack overlapping values. (occasionally happens in CAN network descriptions.)

This was created with AI assistance. I went over all hunks and ensured to understand each of them. (I also found and fixed a few minor issues. As usual, all the good stuff is from the AI, while all bugs are mine ;))

[eerimoq]

negative padding is a bit odd, hmmm... maybe a new character "seeking" in the bit stream?

[andlaus]

IMO negative padding is not too difficult to grasp as a concept, but I also don't mind using a separate format character for this. how about q? (next character after p; s (seek), b (back) and r (reverse) are already taken.)

[andlaus]

okay, I added a patch using a new format letter q for this.

[eerimoq]

Claude and I think these will give wrong result.

buf = bytearray(2)
pack_into('u8q8p4', buf, 0, 0xff, fill_padding=False)
# expected: b'\xff\x00'  (padding leaves bits untouched)
# I think this gives: b'\x0f\x00'

buf = bytearray(b'\xff\xff\xff')
pack_into('u8q4u8u4', buf, 0, 0, 0, 0)
# expected: b'\x00\x00\xff'
# I think this gives: b'\x00\x0f\xff'

[andlaus]

I think this is a matter of definition: the p format is defined as padding filled up with zeros, i.e., as currently implemented (and documented) it overwrites the affected bits. I can also introduce a "seek forward" format character in addition to "seek backwards". maybe q for seek forward (add zeros if the buffer is too short) and Q for seek backward (raise an exception if attempting to seek before the first bit)?

[andlaus]

I think this gives: b'\x00\x0f\xff'

my python seems to disagree:

>>> import bitstruct
>>> buf = bytearray(b'\xff\xff\xff')
>>> bitstruct.pack_into('u8q4u8u4', buf, 0, 0, 0, 0)
>>> buf
bytearray(b'\x00\x00\xff')

[andlaus]

okay, this was a bitstruct.c-specific issue due to how the unaffected bits of the first and last byte of a format where preserved. (It did not work correctly if the first and last bytes was the same byte.) Fixed, thanks for the catch!

[eerimoq]

I'm not sure about this. it adds complexity and makes performance a little worse. how important is this feature?

[andlaus]

it is a long-standing missing feature in cantools which regularly comes up. Recently, a pull request has been opened to implement support for overlapping signals, but this is quite involved (multiple layers of codecs per PDU), and it would an order of magnitude in complexity if there was a way to seek backwards using bitstruct. If you don't like the feature, just close this PR and we'll get cantools#825 merged, if you're not opposed in principle, I'll invest the time to beat bitstruct.c into shape (the problem is that the bitstream writer functions sometimes override the output bits, and sometimes they bitwise-OR it with the value).

[eerimoq]

have you implemented changes in cantools to use bitstruct that can seek backwards? I just want to make sure we don't make changes to bitstruct that in the end did not fit your use case.

[andlaus]

have you implemented changes in cantools to use bitstruct that can seek backwards?

I've asked my LLM to do it: cantools/cantools@17182d0 . while it is not too many fewer lines of code changed than cantools/cantools#825, IMO the result is conceptionally simpler and I suppose it is more performant if overlapping signals are present.

Anyway, no bad blood from my side if you prefer not to have this feature...

[eerimoq]

I suggest not making this change to bitstruct right now, and possibly continue on it later on after evaluation of other solution.

[andlaus]

ok, let's close this PR then. thanks for your feedback!

[eerimoq]

thanks