]> sigrok.org Git - libsigrokdecode.git/commitdiff
i2c: improve reliability of bitrate estimation (throughput, meta)
authorGerhard Sittig <redacted>
Mon, 17 Jul 2023 16:37:37 +0000 (18:37 +0200)
committerGerhard Sittig <redacted>
Tue, 18 Jul 2023 19:09:40 +0000 (21:09 +0200)
The I2C decoder used to track the bitrate of the observed traffic
(number of address and data bits in a transfer). It's uncertain where
this output went (is meta still supported, are applications using it?),
and it appears to not be covered in tests. Improve the logic still.

Adjust the location of the emitted annotation. It used to start at the
most recently observed data byte, which looks suspicious. Output was
attempted when STOP was seen, even if the start was not observed. The
calculation dropped data before a repeated start. The implementation
kept data around after it became obsolete.

Break a long formula across several text lines. Use the fact that Python
division yields floating point results.

Add a comment in the ACK/NAK bit handler. It references data which was
gathered when accumulating data bits. Could be acceptable but must be
remained aware of.

decoders/i2c/pd.py

index 70795c034efbe7235ef77d075904319b1cb7e6a6..850b204db31a38ed1e647b7f7746731cb2780b50 100644 (file)
@@ -142,9 +142,12 @@ class Decoder(srd.Decoder):
 
     def handle_start(self, pins):
         self.ss, self.es = self.samplenum, self.samplenum
-        self.pdu_start = self.samplenum
-        self.pdu_bits = 0
-        cmd = 'START REPEAT' if self.is_repeat_start else 'START'
+        if self.is_repeat_start:
+            cmd = 'START REPEAT'
+        else:
+            cmd = 'START'
+            self.pdu_start = self.samplenum
+            self.pdu_bits = 0
         self.putp([cmd, None])
         cls, texts = proto[cmd][0], proto[cmd][1:]
         self.putx([cls, texts])
@@ -254,6 +257,9 @@ class Decoder(srd.Decoder):
 
     def get_ack(self, pins):
         scl, sda = pins
+        # NOTE! Re-uses the last data bit's width for ACK/NAK as well.
+        # Which might be acceptable because this decoder implementation
+        # only gets to handle ACK/NAK after all DATA BITS were seen.
         self.ss, self.es = self.samplenum, self.samplenum + self.bitwidth
         cmd = 'NACK' if (sda == 1) else 'ACK'
         self.putp([cmd, None])
@@ -272,10 +278,14 @@ class Decoder(srd.Decoder):
 
     def handle_stop(self, pins):
         # Meta bitrate
-        if self.samplerate:
-            elapsed = 1 / float(self.samplerate) * (self.samplenum - self.pdu_start + 1)
+        if self.samplerate and self.pdu_start:
+            elapsed = self.samplenum - self.pdu_start + 1
+            elapsed /= self.samplerate
             bitrate = int(1 / elapsed * self.pdu_bits)
-            self.put(self.ss_byte, self.samplenum, self.out_bitrate, bitrate)
+            ss, es = self.pdu_start, self.samplenum
+            self.put(ss, es, self.out_bitrate, bitrate)
+            self.pdu_start = None
+            self.pdu_bits = 0
 
         cmd = 'STOP'
         self.ss, self.es = self.samplenum, self.samplenum