-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Have Device.write
accept a full byte sequence, not just a single byte
#79
Changes from all commits
69f1127
d5e0471
90a25f0
f7a490b
bd843c1
bba23e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
from typing import List, Optional | ||
|
||
import pytest | ||
|
||
from pyvisa_sim import common | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"data, mask, send_end, want", | ||
[ | ||
(b"1234", None, False, b"1234"), | ||
(b"41234", 3, False, b"40004"), | ||
# Can't figure this one out. Getting ValueError: bytes must in in range(0, 256) | ||
# If I knew more about the purpose of `iter_bytes` then maybe I could | ||
# reconcile it, but I don't have time to investigate right now. | ||
pytest.param( | ||
b"1234", | ||
3, | ||
True, | ||
b"1234", | ||
marks=pytest.mark.xfail( | ||
reason=( | ||
"ValueError bytes must be in range(0, 256). TODO: figure" | ||
" out correct 'want' value." | ||
) | ||
), | ||
), | ||
Comment on lines
+13
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for uncovering a bug ! iter_bytes is meant to clip values to the right number of bits (since serial may use from 5 to 8 bits). When send_end is True the highest data bits should only be set when the message is over. 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). 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. Can you make the change or do you prefer me to take care of it ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll have to take a deeper dive to make sure I grok it. I've opened #80 to track, but I don't think that fixing the bug should block this PR. I'll take a stab at it when I can. If I can't figure it out I'll bounce it back to you 😆 |
||
], | ||
) | ||
def test_iter_bytes( | ||
data: bytes, mask: Optional[int], send_end: bool, want: List[bytes] | ||
) -> None: | ||
got = b"".join(common.iter_bytes(data, mask=mask, send_end=send_end)) | ||
assert got == want | ||
|
||
|
||
def test_iter_bytes_with_send_end_requires_mask() -> None: | ||
with pytest.raises(ValueError): | ||
# Need to wrap in list otherwise the iterator is never called. | ||
list(common.iter_bytes(b"", mask=None, send_end=True)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
# -*- coding: utf-8 -*- | ||
import pyvisa | ||
|
||
from pyvisa_sim.sessions import serial | ||
|
||
serial.SerialInstrumentSession | ||
|
||
|
||
def test_serial_write_with_termination_last_bit(resource_manager): | ||
instr = resource_manager.open_resource( | ||
"ASRL4::INSTR", | ||
read_termination="\n", | ||
write_termination="\r\n", | ||
) | ||
|
||
# Ensure that we test the `asrl_end` block of serial.SerialInstrumentSession.write | ||
instr.set_visa_attribute( | ||
pyvisa.constants.ResourceAttribute.asrl_end_out, | ||
pyvisa.constants.SerialTermination.last_bit, | ||
) | ||
|
||
# There's a bug (maybe?) in common.iter_bytes that we want to avoid for now. | ||
instr.set_visa_attribute( | ||
pyvisa.constants.ResourceAttribute.send_end_enabled, | ||
pyvisa.constants.VI_FALSE, | ||
) | ||
|
||
instr.write("*IDN?") | ||
assert instr.read() == "SCPI,MOCK,VERSION_1.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changelog updated. Not sure how you like to run things. I typically build out an "Unreleased" section during WIP and then rename the section to the release version.
LMK if you want something else done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will do