Skip to content
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

Fixed bug in USBTMC code which caused issues when packets were longer than max usb packet size #449

Merged
merged 7 commits into from
Oct 1, 2024
10 changes: 8 additions & 2 deletions pyvisa_py/protocols/usbtmc.py
Original file line number Diff line number Diff line change
Expand Up @@ -479,13 +479,19 @@
try:
resp = raw_read(recv_chunk + header_size + max_padding)
response = BulkInMessage.from_bytes(resp)
received.extend(response.data)

Check warning on line 482 in pyvisa_py/protocols/usbtmc.py

View check run for this annotation

Codecov / codecov/patch

pyvisa_py/protocols/usbtmc.py#L482

Added line #L482 was not covered by tests
while len(resp) == self.usb_recv_ep.wMaxPacketSize:
# USBTMC Section 3.3 specifies that the first usb packet
# must contain the header. the remaining packets do not need
# the header the message is finished when a "short packet"
# is sent (one whose length is less than wMaxPacketSize)
resp = raw_read(recv_chunk + header_size + max_padding)
received.extend(resp)

Check warning on line 489 in pyvisa_py/protocols/usbtmc.py

View check run for this annotation

Codecov / codecov/patch

pyvisa_py/protocols/usbtmc.py#L488-L489

Added lines #L488 - L489 were not covered by tests
except (usb.core.USBError, ValueError):
# Abort failed Bulk-IN operation.
self._abort_bulk_in(self._btag)
raise

received.extend(response.data)

# Detect EOM only when device sends all expected bytes.
if len(response.data) >= response.transfer_size:
eom = response.transfer_attributes & 1
Expand Down
49 changes: 49 additions & 0 deletions pyvisa_py/usb.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,55 @@
)
return out

def read(self, count: int) -> Tuple[bytes, StatusCode]:

Check warning on line 309 in pyvisa_py/usb.py

View check run for this annotation

Codecov / codecov/patch

pyvisa_py/usb.py#L309

Added line #L309 was not covered by tests
"""Reads data from device or interface synchronously.

Corresponds to viRead function of the VISA library.

Parameters
-----------
count : int
Number of bytes to be read.

Returns
-------
bytes
Data read from the device
StatusCode
Return value of the library call.

"""

def _usb_reader():

Check warning on line 328 in pyvisa_py/usb.py

View check run for this annotation

Codecov / codecov/patch

pyvisa_py/usb.py#L328

Added line #L328 was not covered by tests
"""Data reader identifying usb timeout exception."""
try:
return self.interface.read(count)
except usb.USBError as exc:

Check warning on line 332 in pyvisa_py/usb.py

View check run for this annotation

Codecov / codecov/patch

pyvisa_py/usb.py#L330-L332

Added lines #L330 - L332 were not covered by tests
if exc.errno in (errno.ETIMEDOUT, -errno.ETIMEDOUT):
raise USBTimeoutException()
raise

Check warning on line 335 in pyvisa_py/usb.py

View check run for this annotation

Codecov / codecov/patch

pyvisa_py/usb.py#L334-L335

Added lines #L334 - L335 were not covered by tests

supress_end_en, _ = self.get_attribute(ResourceAttribute.suppress_end_enabled)

Check warning on line 337 in pyvisa_py/usb.py

View check run for this annotation

Codecov / codecov/patch

pyvisa_py/usb.py#L337

Added line #L337 was not covered by tests

if supress_end_en:
raise ValueError(

Check warning on line 340 in pyvisa_py/usb.py

View check run for this annotation

Codecov / codecov/patch

pyvisa_py/usb.py#L340

Added line #L340 was not covered by tests
"VI_ATTR_SUPPRESS_END_EN == True is currently unsupported by pyvisa-py"
)

term_char, _ = self.get_attribute(ResourceAttribute.termchar)
term_char_en, _ = self.get_attribute(ResourceAttribute.termchar_enabled)

Check warning on line 345 in pyvisa_py/usb.py

View check run for this annotation

Codecov / codecov/patch

pyvisa_py/usb.py#L344-L345

Added lines #L344 - L345 were not covered by tests

return self._read(
_usb_reader,
count,
lambda current: False, # USBTMC can return partial message (i.e.,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the end indicator checker does not care about the termination character, only about the fact that the underlying hardware signaled it is done transmitting a message. For GPIB, this is done by checking the state of hardware line, for USB it is always trivially true since by construction a BulkIn message is complete when received. Using this kind of notification is more efficient than checking for term chars.

The _read implementation takes care of ensuring we read till a term char if we were requested to do so. Actually it is at this layer that support for suppress_end is implemented so I would appreciate if while removing this implementation, you could also get rid of the error forbidding the use of suppress_end.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comment. I didn't fully understand what _read was meant to do. I think I have fixed the issue and also allowed suppress_end_en to be True in my lastest commit.

# before the term_char) or have trailing zeros
supress_end_en,
term_char,
term_char_en,
USBTimeoutException,
)


@Session.register(constants.InterfaceType.usb, "RAW")
class USBRawSession(USBSession):
Expand Down
Loading