Re: [sigrok-devel] [PATCH 201/204] uart: unbreak putbin() for 9bit data, use two bytes per UART frame

2016-10-23 Thread Gerhard Sittig
On Sun, Oct 23, 2016 at 22:42 +0200, Uwe Hermann wrote:
> 
> On Sun, Oct 16, 2016 at 06:25:26PM +0200, Gerhard Sittig wrote:
> > Stick with the "one data byte per UART frame" approach for frames which
> > carry 5 to 8 data bits.  But send two bytes per UART frames (big endian
> > representation) for configurations with 9 data bits.
> [ ... ]
> 
> Yup, agreed. I had a similar patch in the queue already, which
> basically does the same thing by using to_bytes(), which is now
> merged.
> 
> (also another one for SPI, which had a similar bug, #686)

Had a look at your commit, looks good to me.  Not open coding the
conversion is preferrable, and especially more maintainable for
larger bit counts (as your dumps commit suggested is the case for
the SPI protocol).


> I updated the sigrok-test repo with a few test-cases to check
> this, but please let me know if there are any regressions with
> this method compared to your approach.

HEAD of master works as expected:

$ for N in 5 6 7 8 9; do
echo "data bits: ${N}"
sigrok-cli -i uart/counter/uart_count_19200_${N}n1.sr \
  -P uart:baudrate=19200:num_data_bits=${N} -B uart=rxtx \
  | hexdump -Cv -n 64
echo
  done

data bits: 5
  1f 00 01 02 03 04 05 06  07 08 09 0a 0b 0c 0d 0e  ||
0010  0f 10 11 12 13 14 15 16  17 18 19 1a 1b 1c 1d 1e  ||
0020  1f 00 01 02 03 04 05 06  07 08 09 0a 0b 0c 0d 0e  ||
0030  0f 10 11 12 13 14 15 16  17 18 19 1a 1b 1c 1d 1e  ||
0040

data bits: 6
  3c 3d 3e 3f 00 01 02 03  04 05 06 07 08 09 0a 0b  |<=>?|
0010  0c 0d 0e 0f 10 11 12 13  14 15 16 17 18 19 1a 1b  ||
0020  1c 1d 1e 1f 20 21 22 23  24 25 26 27 28 29 2a 2b  | !"#$%&'()*+|
0030  2c 2d 2e 2f 30 31 32 33  34 35 36 37 38 39 3a 3b  |,-./0123456789:;|
0040

data bits: 7
  7c 7d 7e 7f 00 01 02 03  04 05 06 07 08 09 0a 0b  ||}~.|
0010  0c 0d 0e 0f 10 11 12 13  14 15 16 17 18 19 1a 1b  ||
0020  1c 1d 1e 1f 20 21 22 23  24 25 26 27 28 29 2a 2b  | !"#$%&'()*+|
0030  2c 2d 2e 2f 30 31 32 33  34 35 36 37 38 39 3a 3b  |,-./0123456789:;|
0040

data bits: 8
  80 81 82 83 84 85 86 87  88 89 8a 8b 8c 8d 8e 8f  ||
0010  90 91 92 93 94 95 96 97  98 99 9a 9b 9c 9d 9e 9f  ||
0020  a0 a1 a2 a3 a4 a5 a6 a7  a8 a9 aa ab ac ad ae af  ||
0030  b0 b1 b2 b3 b4 b5 b6 b7  b8 b9 ba bb bc bd be bf  ||
0040

data bits: 9
  01 f4 01 f5 01 f6 01 f7  01 f8 01 f9 01 fa 01 fb  ||
0010  01 fc 01 fd 01 fe 01 ff  00 00 00 01 00 02 00 03  ||
0020  00 04 00 05 00 06 00 07  00 08 00 09 00 0a 00 0b  ||
0030  00 0c 00 0d 00 0e 00 0f  00 10 00 11 00 12 00 13  ||
0040


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.

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
sigrok-devel mailing list
sigrok-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sigrok-devel


Re: [sigrok-devel] [PATCH 201/204] uart: unbreak putbin() for 9bit data, use two bytes per UART frame

2016-10-23 Thread Uwe Hermann
Hi,

On Sun, Oct 16, 2016 at 06:25:26PM +0200, Gerhard Sittig wrote:
> Stick with the "one data byte per UART frame" approach for frames which
> carry 5 to 8 data bits.  But send two bytes per UART frames (big endian
> representation) for configurations with 9 data bits.
> 
> This addresses Bug 708, passing values greater than 255 to the bytes()
> Python routine broke execution of the decoder.  The proposed patch to
> replace bytes() by bin() is not desirable as it generates variable width
> output.  Using format "{:09b}" is not desirable either as it's rather
> verbose for a binary output stream, and (rather unexpectedly) switches
> from a byte stream output to ASCII characters to represent individual
> bits.  Switching from one byte to two bytes (and only when required) is
> the least intrusive approach, i.e. the least of all available evils.
> Generating 16bit output data for input data which exceeds the 8bit space
> should be most acceptable.

Yup, agreed. I had a similar patch in the queue already, which basically
does the same thing by using to_bytes(), which is now merged.

(also another one for SPI, which had a similar bug, #686)

I updated the sigrok-test repo with a few test-cases to check this, but
please let me know if there are any regressions with this method
compared to your approach.


Thanks, Uwe.
-- 
http://hermann-uwe.de | http://randomprojects.org | http://sigrok.org

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
sigrok-devel mailing list
sigrok-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sigrok-devel


[sigrok-devel] [PATCH 201/204] uart: unbreak putbin() for 9bit data, use two bytes per UART frame

2016-10-16 Thread Gerhard Sittig
Stick with the "one data byte per UART frame" approach for frames which
carry 5 to 8 data bits.  But send two bytes per UART frames (big endian
representation) for configurations with 9 data bits.

This addresses Bug 708, passing values greater than 255 to the bytes()
Python routine broke execution of the decoder.  The proposed patch to
replace bytes() by bin() is not desirable as it generates variable width
output.  Using format "{:09b}" is not desirable either as it's rather
verbose for a binary output stream, and (rather unexpectedly) switches
from a byte stream output to ASCII characters to represent individual
bits.  Switching from one byte to two bytes (and only when required) is
the least intrusive approach, i.e. the least of all available evils.
Generating 16bit output data for input data which exceeds the 8bit space
should be most acceptable.

Signed-off-by: Gerhard Sittig 
---
 decoders/uart/pd.py | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/decoders/uart/pd.py b/decoders/uart/pd.py
index 151cae424fef..b78c617af978 100644
--- a/decoders/uart/pd.py
+++ b/decoders/uart/pd.py
@@ -285,11 +285,18 @@ def get_data_bits(self, rxtx, signal):
 elif f == 'bin':
 self.putx(rxtx, [rxtx, [bin(b)[2:].zfill(8)]])
 
-self.putbin(rxtx, [rxtx, bytes([b])])
-self.putbin(rxtx, [2, bytes([b])])
+formatted = self.format_bytes(b)
+self.putbin(rxtx, [rxtx, formatted])
+self.putbin(rxtx, [2, formatted])
 
 self.databits[rxtx] = []
 
+def format_bytes(self, b):
+"""Format data to either one or two bytes (5-8, and 9 bits)."""
+bits = self.options['num_data_bits']
+data = [ b >> 8, b & 0xff, ] if bits > 8 else [ b, ]
+return bytes(data)
+
 def get_parity_bit(self, rxtx, signal):
 # If no parity is used/configured, skip to the next state immediately.
 if self.options['parity_type'] == 'none':
-- 
1.9.1

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
sigrok-devel mailing list
sigrok-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sigrok-devel