Bug in iter_bytes

[dougthor42]

See #79 (comment)

[dougthor42]

@MatthieuDartiailh I'm taking a look at this and I'm not sure I understand that you've said. Let me break it down to make sure I've got things right - please correct me where I'm wrong:

iter_bytes is meant to clip values to the right number of bits (since serial may use from 5 to 8 bits).

Sounds reasonable. If we send data that requires 10 bits to represent, clip it to 8 bits (or 5 or whatever mask is).

   data:  0b 0011 1111 1100   # eg: something non-ascii
clipped:  0b      1111 1100

When send_end is True the highest data bits should only be set when the message is over.

"should only be set": Did you mean "should only be sent"? Assuming you meant "sent", you mean we should do this:

Data Index	Data	Sent Value (send_end=True)	Sent Value (send_end=False)
0	0b0011 1111 1100	0b1111 1100	0b1111 1100
1	0b0011 1111 1100	0b1111 1100	0b1111 1100
2	0b0011 1111 1100	0b1111 1100	0b1111 1100
3 (last)	0b0011 1111 1100	0b0011 1111 1100	0b1111 1100

The problem here is that ~ gives us a signed binary number. So basically the function is bugged (and it is annoying since it is duplicated in pyvisa-py).

Yeah ~ is "invert" which is defined as -(x+1). If I'm correct in all of the above, then it sounds like we might want AND with 2**mask - 1:

   mask: 8  # eg: "number of bits to use"
2**mask: 0b 1 0000 0000

          data:  0b 0011 1111 1100
&  2**mask - 1:  0b      1111 1111
        result:  0b      1111 1100

To make it less error prone it is we could refactor it to take the number of data bits and send_end. That way we could write out the mask directly.

Do you mean this?

def iter_bytes(data: bytes, max_bits: int = 8, send_end: bool = False) -> Iterator(bytes):
    """Clip values to the correct number of bits (serial may use from 5 to 8 bits).

    Parameters
    ----------
    data : The data to send
    max_bits: Truncate each character to this number of least-significant bits.
    send_end: If True, send the final character with all bits (do not truncate).
        Otherwise, truncate the final character to max_bits.
    """

I might want to rename send_end to something else that answers the question "should we truncate the final character to max_bits"?

[MatthieuDartiailh]

The behavior is meant to match the IVI specification. I attach links to both the IVI specification and NI documentation that details the expectation both for data bits and for send end. Let me know if this clears things.

https://www.ivifoundation.org/downloads/Architecture%20Specifications/vpp43_2022-05-19.pdf

https://www.ni.com/docs/en-US/bundle/ni-visa/page/ni-visa/vi_attr_asrl_data_bits.html

https://www.ni.com/docs/en-US/bundle/ni-visa/page/ni-visa/vi_attr_asrl_end_out.html

[dougthor42]

Thanks for those links. It sure sounds like what I described matches with those specs. I'll whip up a PR.