-
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
Merged
MatthieuDartiailh
merged 6 commits into
pyvisa:main
from
dougthor42:fix-logging-one-char
Apr 8, 2023
Merged
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
69f1127
Add test_common.py
dougthor42 d5e0471
Add test_serial.py
dougthor42 90a25f0
Add test for logging
dougthor42 f7a490b
Have devices.Device.write accept a full byte sequence, not just a sin…
dougthor42 bd843c1
Different way of skipping bad test.
dougthor42 bba23e0
Update changelog
dougthor42 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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." | ||
) | ||
), | ||
), | ||
], | ||
) | ||
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)) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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" |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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 comment
The 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 😆