pyseabreeze: OBPProtocol.receive docstring is incorrect

[ptapping]

spectrometer and system information

• model: (FX, and other models which use OBP protocol)
• operating system: (Ubuntu 20.04 64 bit)
• python version: (3.8.5)
• python-seabreeze version: (current master)
• installed-via: (python setup.py install)

current problem

Attempted to add Ocean FX support to pyseabreeze, but the communications were timing out. Seemed like messages or responses might be incorrect sizes. Looked at code and saw this in the pyseabreeze.protocol.OBP.receive() method:

def receive(self, size=None, timeout_ms=None, **kwargs):
        """receive data from the spectrometer
        Parameters
        ----------
        size : int, optional
            number of bytes to receive. if `None` (default) uses the
            default size as specified in the transport layer.
        timeout_ms : int, optional
            the timeout after which the transport layer should error.
            `None` means no timeout (default)
        kwargs :
            ignored and only present to provide compatible caller interfaces
        Returns
        -------
        data : str
            data returned from the spectrometer
        """
        response = self.transport.read(size=64, timeout_ms=timeout_ms)

Clearly the size parameter is not doing what it says it should do...

Not sure if it's related to my problem, but worth raising the issue...

[ap--]

Hi @ptapping

this is an issue with the docstring for the OBPProtocol. The size parameter isn't used in the receive method.
The first read of 64bytes is enough to parse the header of the reply sent by the spectrometer and determine if the spectrometer needs to read more data.

python-seabreeze/src/seabreeze/pyseabreeze/protocol.py

Lines 294 to 345 in ff806a8

	def receive(self, size=None, timeout_ms=None, **kwargs):
	"""receive data from the spectrometer
	Parameters
	----------
	size : int, optional
	number of bytes to receive. if `None` (default) uses the
	default size as specified in the transport layer.
	timeout_ms : int, optional
	the timeout after which the transport layer should error.
	`None` means no timeout (default)
	kwargs :
	ignored and only present to provide compatible caller interfaces
	Returns
	-------
	data : str
	data returned from the spectrometer
	"""
	response = self.transport.read(size=64, timeout_ms=timeout_ms)
	try:
	remaining_bytes, checksum_type = self._check_incoming_message_header(
	response[:44]
	)
	except SeaBreezeError:
	# empty buffer if error raised
	self.transport.read(size=None, timeout_ms=500)
	raise
	length_payload_footer = remaining_bytes
	# we already received some data
	remaining_bytes -= len(response[44:])
	if remaining_bytes > 0:
	response += self.transport.read(size=remaining_bytes, timeout_ms=timeout_ms)
	if length_payload_footer != len(response[44:]):
	raise SeaBreezeError(
	"remaining packet length mismatch: {:d} != {:d}".format(
	remaining_bytes, len(response[44:])
	)
	)
	checksum = self._check_incoming_message_footer(response[-20:])
	if (
	checksum_type == self.OBP.CHECKSUM_TYPE_MD5
	and checksum != hashlib.md5(response[:-20]).digest()
	):
	warnings.warn(
	"WARNING OBP: The checksums differ, but we ignore this for now."
	)
	return self._extract_message_data(response)

is basically a minimal implementation of

python-seabreeze/src/libseabreeze/src/vendors/OceanOptics/protocols/obp/exchanges/OBPTransaction.cpp

Lines 74 to 222 in ff806a8

	vector<byte> *OBPTransaction::queryDevice(TransferHelper *helper,
	unsigned int messageType,
	vector<byte> &data)
	{
	int flag = 0;
	vector<byte> *bytes = NULL;
	vector<byte> *fullVector = NULL;
	OBPMessage *message = new OBPMessage();
	OBPMessage *response = NULL;
	message->setMessageType(messageType);
	/* Need a copy of the input data that can be given to the message. */
	/* Note: copy will be deleted by message when appropriate. */
	message->setData(new vector<byte>(data));
	try
	{
	bytes = message->toByteStream();
	flag = helper->send(*bytes, (unsigned) bytes->size());
	if(((unsigned int)flag) != bytes->size())
	{
	/* FIXME: retry, throw exception, something here */
	}
	}
	catch (const BusException &be)
	{
	if(NULL != bytes)
	{
	delete bytes;
	}
	delete message;
	string error("Failed to write to bus.");
	/* FIXME: previous exception should probably be bundled up into the new exception */
	/* FIXME: there is probably a more descriptive type for this than ProtocolException */
	throw ProtocolException(error);
	}
	delete message;
	delete bytes;
	bytes = NULL;
	try {
	/* Read the 64-byte OBP header. This may indicate that more data
	* must be absorbed afterwards.
	*/
	bytes = new vector<byte>(64);
	flag = helper->receive(*bytes, (unsigned) bytes->size());
	if(((unsigned int)flag) != bytes->size())
	{
	/* FIXME: retry, throw exception, something here */
	}
	/* Parse out the header and see if there is an extended payload. */
	try
	{
	response = OBPMessage::parseHeaderFromByteStream(bytes);
	}
	catch (const IllegalArgumentException &iae)
	{
	response = NULL;
	}
	if(NULL == response || true == response->isNackFlagSet() || response->getMessageType() != messageType)
	{
	if(NULL != bytes)
	{
	delete bytes;
	}
	if(NULL != response)
	{
	char message[64];
	if (response->getMessageType() == messageType)
	{
	unsigned short flags = (*response).getFlags();
	snprintf(message, sizeof(message), "OBP Flags indicated an error: %x", flags);
	}
	else
	{
	snprintf(message, sizeof(message), "Expected message type 0x%x, but got %x", messageType, response->getMessageType());
	}
	delete response;
	throw ProtocolException(message);
	}
	/* There may be a legitimate reason to not return a message
	* (e.g. tried to read an unprogrammed value). Just return
	* NULL here instead of throwing an exception and let the
	* caller figure it out.
	*/
	return NULL;
	}
	unsigned int bytesToRead = response->getBytesRemaining() - 20; /* omit footer and checksum */
	if(bytesToRead > 0) {
	fullVector = new vector<byte>(bytesToRead + bytes->size());
	/* Safely stl::copy() the header into a full-sized block and
	* delete the existing buffer
	*/
	vector<byte>::iterator iter = copy(bytes->begin(), bytes->end(), fullVector->begin());
	delete bytes;
	bytes = NULL;
	/* TransferHelper expects a vector, so create a new one for it */
	vector<byte> *remainder = new vector<byte>(bytesToRead);
	flag = helper->receive(*remainder, (unsigned) remainder->size());
	if(((unsigned int)flag) != bytesToRead) {
	/* FIXME: retry, throw exception, something here */
	}
	copy(remainder->begin(), remainder->end(), iter);
	delete remainder;
	} else {
	fullVector = bytes;
	bytes = NULL; /* To prevent double deletion */
	}
	/* Delete and recreate response using the full message */
	delete response;
	try {
	response = OBPMessage::parseByteStream(fullVector);
	} catch (IllegalArgumentException &iae) {
	response = NULL;
	}
	delete fullVector;
	fullVector = NULL;
	} catch (BusException &be) {
	if(NULL != bytes) {
	delete bytes;
	}
	if(NULL != fullVector) {
	delete fullVector;
	}
	string error("Failed to read from bus.");
	/* FIXME: previous exception should probably be bundled up into the new exception */
	/* FIXME: there is probably a more descriptive type for this than ProtocolException */
	throw ProtocolException(error);
	}
	if(NULL == response) {
	/* This could happen if the footer or checksum failed for
	* some reason, but that would be very unusual. This can only happen
	* if the header was already verified, but there was some error in the
	* rest of the payload.
	*/
	string error("Failed to parse extended message");
	throw ProtocolException(error);
	}
	/* Make a copy of the response buffer so response can be deleted */
	bytes = new vector<byte>(*response->getData());
	delete response;
	return bytes;
	}

This should be better documented...

Cheers,
Andreas 😃