From: Gerhard Sittig <redacted>
Date: Mon, 17 Jul 2023 16:37:37 +0000 (+0200)
Subject: i2c: improve reliability of bitrate estimation (throughput, meta)
X-Git-Url: https://sigrok.org/gitweb/?a=commitdiff_plain;h=5eb664089f9123a91841f66535ffb1689d9ddd98;hp=01416b9810ceb759e3088bad174d89621704a210;p=libsigrokdecode.git

i2c: improve reliability of bitrate estimation (throughput, meta)

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.
---

diff --git a/decoders/i2c/pd.py b/decoders/i2c/pd.py
index 70795c0..850b204 100644
--- a/decoders/i2c/pd.py
+++ b/decoders/i2c/pd.py
@@ -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