Erratic "falling" interrupt on rising edge (bad cabling) => missing commands

[stromnet]

First of all, not sure if this project is alive or not but I'm still using it for some stuff. In any case wanted to document my findings & experiments here.

I've had some issues with a new device, where it in test network worked but in real network failed. The master is an ESP8266 which bitbangs a PIO. Originally it was pulled-up to 3.3V, with the AVR running on 5V, which had worked good with 2 mega8/mega88 slaves so far. But not with this 3rd one. The wiring is however non-optimal, star topology with 2 long legs, and this new CPU on a third shorter leg.

First attempt of a change was to introduce a simple level-shifter, as per https://www.analog.com/en/design-notes/how-to-level-shift-1wire-systems.html, so that bus was 5V. 2n7000 was used, and a resistor pull-up after the level-shifter to 5V was 3.9k.
This did not help (not that I'd really expect it to, but was nice to get fixed anyway, giving 3.3V logical high, via some cable losses, to 5V CPU is not really guaranteed to work. mega88 has VCC*0.6 = 3V which I was very close to at remote end).

The problem was reproduced in a much smaller network: 50cm of 4-wire cable with 5v,gnd,1w -> star split to 4-wire cable of a few meters with a DS18B20, and other side of split, ~0.5m cat5 to my SUT.

The issue observed (at least in reproductoin) is, during the RISING edge at end of some bits, the CPU triggers the interrupt which at the time is configured for FALLING edge.

With dbgpin & uart debug enabled, it was seen that the MATCH ROM command was successfully received, but that immediately upon receiving ROM code to match against, it was rejected as "not mine".

Looking at the dbgpin vs the 1w bus, it clearly shows that the INT0 isr is triggered during rising. First a full MATCH ROM image:

Here is a good write-1 (first high on DBG pin is the ISR, second high low high is the Timer sampling the line):

Here is a good write-0:

Here is the bad write-0, the last bit of the MATCH ROM command. See the rising edge, where it seems to trigger a interrupt again (DBG pin goes high, followed by a sampling in the middle of what is the recovery period).

This happens to be the last bit of the MATCH ROM almost every time, I'm guessing some other slave is causing that erratic pull-up disturbance. In any case, the MCU (mega88a-pu @ 8Mhz on crappy breadboard) seems to react to that noise as a falling edge.

One quick-fix that helped: add some resistance in series with the 1w input pin (tested with 150 ohm, seems to work just fine).

Another thing attempted, decrease the pull-up from 3.9k to 2.2k or 1.1k, but did not make any difference.

So, question is if this could easily be fixed in code. Brief idea, is that once it has sampled a bit, it could SET_RISING(). And when it has risen, it just sets SET_FALLING() again. The brief delay in reacting on this should be enough to miss out on the ringing, but still fast enough to SET_FALLING during recovery time, so we can catch the next bit.

I made PoC (see below), where the ISR now fires during rising edge, in case a write-0 was seen:

The ISR takes 2.6us to execute here (with debug pin & port, 2.4uS with only pin). At first glance this seems to work pretty good!

Will leave it for a while at bench and see how it works out. Then we'll see if this helps on the device in the real network.

One problem though: if I enable uart_debug, it is too slow on 8Mhz. The poll_xmit code disables interrupts when it has byte to send, and this seemingly causing delays for the falling edge interrupt on the second bit as seen here.

(verified with dbg port that it was indeed in the uart xmit code at the time).
However, that could probably as well happen without my changes?

Current experimental code:

diff --git a/onewire.c b/onewire.c
index 39f9935..b1382cb 100644
--- a/onewire.c
+++ b/onewire.c
@@ -126,8 +128,12 @@ xmit_any(uint8_t val, uint8_t len)
 {
        wait_complete('w');
        cli();
-       if(mode == OWM_READ || mode == OWM_IDLE)
+       if(mode == OWM_READ || mode == OWM_IDLE) {
                mode = OWM_WRITE;
+               // ensure we do not wait for a rising edge
+               SET_FALLING();
+               waiting_rising = 0;
+       }
        if (mode != OWM_WRITE || xmode < OWX_RUNNING) {
                // DBG_P("\nErr xmit ");
                DBG(0x28);
@@ -400,6 +409,7 @@ void set_idle(void)
        CLEAR_LOW();
        DIS_TIMER();
        SET_FALLING();
+       waiting_rising = 0;
        EN_OWINT();
        SREG = sreg;
 }
@@ -455,10 +466,18 @@ TIMER_INT
                xmode = OWX_SELECT;
                break;
        case OWM_READ:
+               DBG(0x09);
                if(lbitp) {
                        //DBG_C(p ? 'B' : 'b');
-                       if (p)  // Set bit if line high 
+                       if (p)  // Set bit if line high
                                cbuf |= lbitp;
+                       else {
+                               // After a read 0, wait for the rising edge.
+                               // It will immediately change to SET_FALLING().
+                               // This ensures we don't accidentally act on a falling edge in noise during the rise.
+                               SET_RISING();
+                               waiting_rising = 1;
+                       }
                        lbitp <<= 1;
                } else {
                        // Overrun!
@@ -537,7 +556,29 @@ void PIN_INT(void)
        // WARNING: No command in here may change the status register!
        DBG_ON();
        asm("     push r24");
-       asm("     push r25");
+       // if waiting_rising==1
+       asm("     lds r24,waiting_rising");
+       asm("     sbrs r24,0");
+       asm("     rjmp .Lm"); //
+       // ...clear waiting_rising
+       asm("     clr r24");
+       asm("     sts waiting_rising,r24");
+       //SET_FALLING();
+#define XSTR(x) #x
+#define STR(s) XSTR(s)
+       asm("     lds r24,%0" :: "i"((int)&EICRA)); // TODO: features.h
+       asm("     cbr r24," STR(1<<ISC00));
+       asm("     sbr r24," STR(1<<ISC01));
+       asm("     sts %0,r24" :: "i"((int)&EICRA));
+#ifdef DBGPORT
+       asm("     ldi r24,0x04"); // DBG(0x04);
+       asm("     out %0,r24" :: "i"(((int)&DBGPORT)-__SFR_OFFSET));
+#endif
+       asm("     pop r24");
+       DBG_OFF();
+       asm("     ret");
+       //... else
+       asm(".Lm: push r25");
        asm("     lds r24,wmode");
 #ifdef DBGPORT
        asm("     out %0,r24" :: "i"(((int)&DBGPORT)-__SFR_OFFSET));
diff --git a/onewire_internal.h b/onewire_internal.h
index 7a9b4e2..6e5c844 100644
--- a/onewire_internal.h
+++ b/onewire_internal.h
@@ -152,6 +152,7 @@ typedef enum {
        OWM_WRITE, // writing some bits
 } mode_t;
 volatile mode_t mode; //state
+volatile uint8_t waiting_rising;
 
 //next high-level state
 typedef enum {