On Thu, 2020-10-01 at 21:57 +0200, Michael Ströder wrote:
> 
> I'm trying to query data from a sound level meter PCE-322A on Linux
> (openSUSE Tumbleweed). Thus I was happy to find out that this device
> seems supported by sigrok. I'm new to sigrok and also have no prior
> experience with the device.
> 
> $ sigrok-cli --driver=pce-322a:conn=/dev/ttyUSB0 -l 5 --show
> [ ... ]
> sr: [00:00.004911] hwdriver: Scan found 1 devices (pce-322a).
> pce-322a - PCE PCE-322A with 1 channel: SPL
> sr: [00:00.004937] device: pce-322a: Opening device instance.
> sr: [00:00.004942] serial: Opening serial port '/dev/ttyUSB0' (flags 1).
> sr: [00:00.005837] serial: Parsing parameters from "115200/8n1".
> sr: [00:00.005855] serial: Setting serial parameters on port /dev/ttyUSB0.
> sr: [00:00.005948] serial: DBG: serial_set_params() rate 115200, 8n1
> sr: [00:00.005964] serial: Wrote 2/2 bytes.
> Failed to open device.

The pce-322a support was introduced in 2016. In 2017 a bug snuck
into the driver source during maintenance and fixing of a serial
layer API use issue. Seems like it went unnoticed until now.
Shows how important it is that users with access to special or
rare hardware help keep and increase the coverage by testing
newer versions, too.

Have massaged the code, but cannot test whether the fix is
effective without access to the hardware. See the attachment.
Would be good if you could verify the operation. Or getting an
ACK from other people's review can help apply the fix in mainline
and thus show up in nightlies.

Also please state if you want your name on the commit message as
the user who helped improve the software by reporting the issue.


virtually yours
Gerhard Sittig
-- 
     If you don't understand or are scared by any of the above
             ask your parents or an adult to help you.
From c6dd857a783ab6437d2cbeb071002e63fdf2f6c0 Mon Sep 17 00:00:00 2001
From: Gerhard Sittig <gerhard.sit...@gmx.net>
Date: Fri, 2 Oct 2020 11:18:57 +0200
Subject: [PATCH] WIP pce-322a: unbreak send_command() return code
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Data was sent to the serial port, and the non-zero positive write length
was mistaken as an error, since it did not match the SR_OK code's value.
This snuck in with commit 379e95c587e1d in 2017-08.

Rephrase the check for successful serial writes in the pce-322a driver.
Pass on error codes from the serial layer in verbatim form. Check for
the exact expected write length and derive SR_ERR_IO upon mismatch. Do
return SR_OK upon success.

Reported-By: Michael Ströder <mich...@stroeder.com>

TODO This is untested beyond compilation, due to lack of hardware access.
---
 src/hardware/pce-322a/protocol.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/hardware/pce-322a/protocol.c b/src/hardware/pce-322a/protocol.c
index ee637e802dcb..ee98c7c61128 100644
--- a/src/hardware/pce-322a/protocol.c
+++ b/src/hardware/pce-322a/protocol.c
@@ -26,6 +26,7 @@ static int send_command(const struct sr_dev_inst *sdi, uint16_t command)
 {
 	struct sr_serial_dev_inst *serial;
 	uint8_t buffer[2];
+	int ret;
 
 	buffer[0] = command >> 8;
 	buffer[1] = command;
@@ -33,13 +34,20 @@ static int send_command(const struct sr_dev_inst *sdi, uint16_t command)
 	if (!(serial = sdi->conn))
 		return SR_ERR;
 
-	return serial_write_blocking(serial, (const void *)buffer, 2, 0);
+	ret = serial_write_blocking(serial, buffer, sizeof(buffer), 0);
+	if (ret < 0)
+		return ret;
+	if ((size_t)ret != sizeof(buffer))
+		return SR_ERR_IO;
+
+	return SR_OK;
 }
 
 static int send_long_command(const struct sr_dev_inst *sdi, uint32_t command)
 {
 	struct sr_serial_dev_inst *serial;
 	uint8_t buffer[4];
+	int ret;
 
 	buffer[0] = command >> 24;
 	buffer[1] = command >> 16;
@@ -49,7 +57,13 @@ static int send_long_command(const struct sr_dev_inst *sdi, uint32_t command)
 	if (!(serial = sdi->conn))
 		return SR_ERR;
 
-	return serial_write_blocking(serial, (const void *)buffer, 4, 0);
+	ret = serial_write_blocking(serial, buffer, sizeof(buffer), 0);
+	if (ret < 0)
+		return ret;
+	if ((size_t)ret != sizeof(buffer))
+		return SR_ERR_IO;
+
+	return SR_OK;
 }
 
 static void send_data(const struct sr_dev_inst *sdi, float sample)
-- 
2.22.0

_______________________________________________
sigrok-devel mailing list
sigrok-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sigrok-devel

Reply via email to