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

Have `Device.write` accept a full byte sequence, not just a single byte #79

Closed dougthor42 closed 1 year ago

dougthor42 commented 1 year ago

Fixes #72.

While working on this, I noticed that this block

https://github.com/pyvisa/pyvisa-sim/blob/2aab58ba092043a89628787729175acbba8695c8/pyvisa_sim/sessions/serial.py#L84-L88

was not touched by unit tests. So I added a test for it, as the proposed change to device.Device.write will impact that code.

And then while creating the test for serial.py, I found that common.iter_bytes was also not covered by tests, so I made a test case for that.

I also add a test to ensure that logging is recording the full string, not just 1 character at a time.

Now that all impacted code is covered by tests, I implemented the minor change to device.Device.write, session.MessageBasedSession.write and serial.SerialInstrumentSession.write to fix the logging issue.

codecov-commenter commented 1 year ago

Codecov Report

Merging #79 (bba23e0) into main (2aab58b) will increase coverage by 1.91%. The diff coverage is 100.00%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main      #79      +/-   ##
==========================================
+ Coverage   82.88%   84.80%   +1.91%     
==========================================
  Files          16       18       +2     
  Lines        1011     1033      +22     
  Branches      153      152       -1     
==========================================
+ Hits          838      876      +38     
+ Misses        135      120      -15     
+ Partials       38       37       -1     
Flag Coverage Δ
unittests 84.80% <100.00%> (+1.91%) :arrow_up:

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

Impacted Files Coverage Δ
pyvisa_sim/devices.py 94.44% <ø> (+0.41%) :arrow_up:
pyvisa_sim/sessions/serial.py 68.75% <100.00%> (+7.21%) :arrow_up:
pyvisa_sim/sessions/session.py 71.95% <100.00%> (-0.34%) :arrow_down:
pyvisa_sim/testsuite/test_all.py 100.00% <100.00%> (ø)
pyvisa_sim/testsuite/test_common.py 100.00% <100.00%> (ø)
pyvisa_sim/testsuite/test_serial.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

dougthor42 commented 1 year ago

@MatthieuDartiailh is there anything blocking this from getting merged?

MatthieuDartiailh commented 1 year ago

I am just slow. Just so that it does not get lost could you add a change log entry and I will merge ?

By curiosity, will you be able to address #80 ?

dougthor42 commented 1 year ago

No worries! I completely understand. And yes I expect to be able to take a look at #80.