pyvisa / pyvisa-sim

A PyVISA backend that simulates a large part of the "Virtual Instrument Software Architecture" (VISA_)
https://pyvisa-sim.readthedocs.io/en/latest/
MIT License
69 stars 39 forks source link

Fix `common.iter_bytes` #81

Closed dougthor42 closed 1 year ago

dougthor42 commented 1 year ago

Fixes #80.

See that Issue for more detail.

codecov-commenter commented 1 year ago

Codecov Report

Merging #81 (7644d02) into main (dccb9ba) will increase coverage by 0.17%. The diff coverage is 95.23%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main      #81      +/-   ##
==========================================
+ Coverage   84.80%   84.97%   +0.17%     
==========================================
  Files          18       18              
  Lines        1033     1058      +25     
  Branches      152      160       +8     
==========================================
+ Hits          876      899      +23     
- Misses        120      121       +1     
- Partials       37       38       +1     
Flag Coverage Δ
unittests 84.97% <95.23%> (+0.17%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pyvisa_sim/testsuite/test_serial.py 100.00% <ø> (ø)
pyvisa_sim/common.py 91.66% <92.85%> (-2.78%) :arrow_down:
pyvisa_sim/sessions/serial.py 68.75% <100.00%> (ø)
pyvisa_sim/testsuite/test_common.py 100.00% <100.00%> (ø)
MatthieuDartiailh commented 1 year ago

I have a slightly different understanding of send_end.

The way I read the specs if we have 7 data bits and send_end is True, then the bit 7 should be true only for the last bytes. In your case it seems you encode send_end in an extra bit compared to the number of allowed data bits.

I checked on my phone (little time this days) so I may have missed something.

dougthor42 commented 1 year ago

Hmm... So you're interpreting "transmit the last character with the eighth bit set" as "8th bit == 1", not "8th bit == whatever value it would have been for that character"

Like this? (7 data bits)

Input send_end=True send_end=False
N/A 7th bit set to 1 for last byte no matter what the original value was.
8th bit always removed b/c data_bits=7.
7th bit set to 0 for last byte no matter what the original value was.
8th bit always removed b/c data_bits=7.
0b1111_1111 0b0111_1111 0b0011_1111
0b0000_1111 0b0100_1111
0b0000_1111
0b1111_1100,
0b1011_1100,
0b1011_1100
0b0111_1100,
0b0011_1100,
0b0111_1100
0b0111_1100,
0b0011_1100,
0b0011_1100

Do I have that correct?

(The below is for my own future reference, emphasis and additions mine)

If it is set to VI_ASRL_END_LAST_BIT (pyvisa.constants.SerialTermination.last_bit), and VI_ATTR_SEND_END_EN (pyvisa.constants.ResourceAttribute.send_end_enabled which is the send_end arg) is set to VI_TRUE (True), the write will send all but the last character with the highest bit clear, then transmit the last character with the highest bit set. For example, if VI_ATTR_ASRL_DATA_BITS is set to 8, the write will clear the eighth bit for all but the last character, then transmit the last character with the eighth bit set. If VI_ATTR_SEND_END_EN is set to VI_FALSE, the write will send all the characters with the highest bit clear.

MatthieuDartiailh commented 1 year ago

Rereading https://www.ni.com/docs/en-US/bundle/ni-visa/page/ni-visa/vi_attr_asrl_end_out.html, I realize I missed one case which calls for taking bool | None as argument:

Does that make sense ?

dougthor42 commented 1 year ago

Yeah I think so. Check out the new test suite and LMK.

MatthieuDartiailh commented 1 year ago

Thanks !

Could you make a similar PR againt PyVISA-py (I have no good solution to avoid the duplication) ?