Description
===========
For some reason with certain TLB-6700 controllers, the QMI driver gets erroneous responses to queries. It was reported that the get_piezo_voltages returns perhaps one time out of five partial controller IDN string without any clear reason. With some closer look at the driver and logging in debug mode, there was no reason to suspect that the driver would have encountered an error and called *IDN? at re-initialization. Thus this likely is a fluke of the controller or its firmware or the hardware driver. We cannot fully close out the possibility that multiple QMI connections to the controller were open, which could also cause issues like this, as the testing was done on otherwise active setup and not all other processes could not be closed.
On another setup with the same model controller, QMI driver, firmware version and hardware drivers, the issue did not appear. On superficial level no differences should be present in the setups regarding the laser controller.
Earlier, a similar issue with response "OK\r\n" to the query, instead of the query value, was fixed by checking the response before trying to parse or cast it. A similar check should be done also now for the sudden *IDN? response. The response, according to the user manual, is of format 'NEW_FOCUS XXXX vYYY mm/dd/yy, SNZZZZ'. We see this, although the response size seems to be capped and the first letters were cut, leaving something like cus TLB-6700 v2.4 31/09/23 SN12345. As the exact model, version, date and serial number can all change, we should make a generic check with f.ex. checking only the last part, starting from the version, but by recognizing only that numbers are present at expected places around the spaces and characters.
Circumvent the issue by checking the response length in _send():
if len(response) > 2 and len(response[-2]) > 0 and response[-2] != "OK":
# The response probably is this. This presumes the last value in list is [''] due to "...\r\n" split.
# But check for sporadic "*IDN?" query response first.
idn_match = # TODO: some kind of regular expression check on the string of response[-2] (or alternative)
if idn_match: # The entry is the invalid response, delete it
del response[-2]
# OK check. Otherwise returns response[0]
if response[-2] != "OK":
return response[-2]
A further improvement could also be made: As the device IDs are "fixed" once the hardware is on the setup, re-initialization and un-initialization should not have ANY effect on the device ID. Now, once the driver is started and the instrument opened, we have the device ID stored in self._device_id. The valid ID range is 0-31. So, instead of setting this in class __init__ as : int = 0, we could do : int = -1. This will be set to the current device's Device ID number in open() which in turns checks this value in _init_device(). Currently, if there is a need to re-initialize the device, this value is obtained again parsing the _get_device_info()
call result, but as the number should be the same as before, this is not necessary.
The proposal is to add an extra check after the self._handle.newp_usb_init_product(self.PRODUCT_ID) call to see if the self._device_id is/has been changed to be in the range 0-31, then all the following steps are not necessary and the function should return directly. Also, the function should set itself the self._device_id and not return it to open() in that case, to make things a bit simpler.
def _init_device(self) -> None:
"""Find and initialize the device ID for the device with the given serial number."""
# Open all USB devices that match the USB product ID.
self._handle.newp_usb_init_product(self.PRODUCT_ID)
if self._device_id in range(32):
return
....
self._device_id = our_device_id
@rpc_method
def open(self) -> None:
"""Open connection to the device controller."""
super().open()
_logger.info(f"Opening connection to {self._name}")
self._init_device()
Description =========== For some reason with certain TLB-6700 controllers, the QMI driver gets erroneous responses to queries. It was reported that the
get_piezo_voltages
returns perhaps one time out of five partial controller IDN string without any clear reason. With some closer look at the driver and logging in debug mode, there was no reason to suspect that the driver would have encountered an error and called*IDN?
at re-initialization. Thus this likely is a fluke of the controller or its firmware or the hardware driver. We cannot fully close out the possibility that multiple QMI connections to the controller were open, which could also cause issues like this, as the testing was done on otherwise active setup and not all other processes could not be closed.On another setup with the same model controller, QMI driver, firmware version and hardware drivers, the issue did not appear. On superficial level no differences should be present in the setups regarding the laser controller.
Earlier, a similar issue with response "OK\r\n" to the query, instead of the query value, was fixed by checking the response before trying to parse or cast it. A similar check should be done also now for the sudden
*IDN?
response. The response, according to the user manual, is of format 'NEW_FOCUS XXXX vYYY mm/dd/yy, SNZZZZ'. We see this, although the response size seems to be capped and the first letters were cut, leaving something likecus TLB-6700 v2.4 31/09/23 SN12345
. As the exact model, version, date and serial number can all change, we should make a generic check with f.ex. checking only the last part, starting from the version, but by recognizing only that numbers are present at expected places around the spaces and characters.Circumvent the issue by checking the response length in
_send()
:A further improvement could also be made: As the device IDs are "fixed" once the hardware is on the setup, re-initialization and un-initialization should not have ANY effect on the device ID. Now, once the driver is started and the instrument opened, we have the device ID stored in
self._device_id
. The valid ID range is 0-31. So, instead of setting this in class__init__
as: int = 0
, we could do: int = -1
. This will be set to the current device's Device ID number inopen()
which in turns checks this value in_init_device()
. Currently, if there is a need to re-initialize the device, this value is obtained again parsing the_get_device_info()
call result, but as the number should be the same as before, this is not necessary.The proposal is to add an extra check after the
self._handle.newp_usb_init_product(self.PRODUCT_ID)
call to see if theself._device_id
is/has been changed to be in the range 0-31, then all the following steps are not necessary and the function should return directly. Also, the function should set itself theself._device_id
and not return it toopen()
in that case, to make things a bit simpler.Modules to be modified
qmi.instruments.newport.tlb_670x.py
Tests to be created/updated
tests.instruments.newport.test_tlb_670x.py
Documentation to be updated
CHANGELOG.md
Hardware
Test on controller if possible.