Merge pull request #4173 from TD-er/bugfix/Dallas_timings_finetune

[Dallas] Fine tune timings for Dallas sensors

src/_P080_DallasIButton.ino

@@ -94,6 +94,11 @@ boolean Plugin_080(uint8_t function, struct EventStruct *event, String& string)
 
       if (validGpio(Plugin_080_DallasPin)) {
         uint8_t addr[8];
+
+        // Explicitly set the pinMode using the "slow" pinMode function
+        // This way we know for sure the state of any pull-up or -down resistor is known.
+        pinMode(Plugin_080_DallasPin, INPUT);
+
         Dallas_plugin_get_addr(addr, event->TaskIndex);
         Dallas_startConversion(addr, Plugin_080_DallasPin, Plugin_080_DallasPin);
 

src/_P100_DS2423_counter.ino

@@ -102,6 +102,12 @@ boolean Plugin_100(uint8_t function, struct EventStruct *event, String& string)
       UserVar[event->BaseVarIndex + 1] = 0;
       UserVar[event->BaseVarIndex + 2] = 0;
 
+      if (validGpio(CONFIG_PIN1)) {
+        // Explicitly set the pinMode using the "slow" pinMode function
+        // This way we know for sure the state of any pull-up or -down resistor is known.
+        pinMode(CONFIG_PIN1, INPUT);
+      }
+
       success = true;
       break;
     }

src/src/Helpers/Dallas1WireHelper.cpp

@@ -8,6 +8,12 @@
 #include "../WebServer/JSON.h"
 
 
+
+// DEBUG code using logic analyzer for timings
+// #define DEBUG_LOGIC_ANALYZER_PIN  27 
+// #define DEBUG_LOGIC_ANALYZER_PIN_ERROR  26
+
+
 // Macros to perform direct access on GPIOs
 // Macros written by Paul Stoffregen
 // See: https://github.com/PaulStoffregen/OneWire/blob/master/util/
@@ -134,15 +140,6 @@ void Dallas_addr_selector_webform_load(taskIndex_t TaskIndex, int8_t gpio_pin_rx
     }
   }
 
-  // The Shelly 1 temp. addon uses separate
-  // input and output pins, and therefor
-  // doesn't switch between input and output
-  // when running.
-  if (gpio_pin_rx != gpio_pin_tx) {
-    pinMode(gpio_pin_rx, INPUT);
-    pinMode(gpio_pin_tx, OUTPUT);
-  }
-
   // find all suitable devices
   std::vector<uint64_t> scan_res;
 
@@ -379,6 +376,14 @@ bool Dallas_readTemp(const uint8_t ROM[8], float *value, int8_t gpio_pin_rx, int
 
   if (!crc_ok)
   {
+#ifdef DEBUG_LOGIC_ANALYZER_PIN_ERROR
+  // Toggle the CRC error pin to make it better visible in the logic analyzer trace
+  static bool error_pin_toggle = false;
+  error_pin_toggle = !error_pin_toggle;
+  DIRECT_pinWrite(DEBUG_LOGIC_ANALYZER_PIN_ERROR, error_pin_toggle ? 1 : 0);
+#endif
+
+
     *value = 0;
     return false;
   }
@@ -626,11 +631,20 @@ uint8_t Dallas_reset(int8_t gpio_pin_rx, int8_t gpio_pin_tx)
 
   noInterrupts();
 
+#ifdef DEBUG_LOGIC_ANALYZER_PIN
+  // DEBUG code using logic analyzer for timings
+  DIRECT_pinWrite(DEBUG_LOGIC_ANALYZER_PIN, 1);
+#endif
+
   if (gpio_pin_rx == gpio_pin_tx) {
     DIRECT_PINMODE_INPUT(gpio_pin_rx);
   } else {
     DIRECT_pinWrite(gpio_pin_tx, 1);
   }
+#ifdef DEBUG_LOGIC_ANALYZER_PIN
+  // DEBUG code using logic analyzer for timings
+  DIRECT_pinWrite(DEBUG_LOGIC_ANALYZER_PIN, 0);
+#endif
   bool success = true;
 
   do // wait until the wire is high... just in case
@@ -647,6 +661,10 @@ uint8_t Dallas_reset(int8_t gpio_pin_rx, int8_t gpio_pin_tx)
   presence_end   = 0;
 
   if (success) {
+#ifdef DEBUG_LOGIC_ANALYZER_PIN
+  // DEBUG code using logic analyzer for timings
+  DIRECT_pinWrite(DEBUG_LOGIC_ANALYZER_PIN, 1);
+#endif
     // The master starts a transmission with a reset pulse,
     // which pulls the wire to 0 volts for at least 480 µs.
     // This resets every slave device on the bus.
@@ -655,6 +673,7 @@ uint8_t Dallas_reset(int8_t gpio_pin_rx, int8_t gpio_pin_tx)
     if (gpio_pin_rx == gpio_pin_tx) {
       DIRECT_PINMODE_OUTPUT(gpio_pin_rx);
     }
+
     delayMicroseconds(480);
 
     if (gpio_pin_rx == gpio_pin_tx) {
@@ -662,6 +681,11 @@ uint8_t Dallas_reset(int8_t gpio_pin_rx, int8_t gpio_pin_tx)
     } else {
       DIRECT_pinWrite(gpio_pin_tx, 1);
     }
+#ifdef DEBUG_LOGIC_ANALYZER_PIN
+  // DEBUG code using logic analyzer for timings
+  DIRECT_pinWrite(DEBUG_LOGIC_ANALYZER_PIN, 0);
+#endif
+
 
     // After that, any slave device, if present, shows that it exists with a "presence" pulse:
     // it holds the bus low for at least 60 µs after the master releases the bus.
@@ -674,7 +698,9 @@ uint8_t Dallas_reset(int8_t gpio_pin_rx, int8_t gpio_pin_tx)
     const uint64_t start = getMicros64();
     int64_t usec_passed  = 0;
 
-    while (usec_passed < 480) {
+    bool waiting_for_presence = true;
+
+    while ((usec_passed < 480) && waiting_for_presence) {
       usec_passed = usecPassedSince(start);
 
       const bool pin_state = !!DIRECT_pinRead(gpio_pin_rx);
@@ -694,6 +720,14 @@ uint8_t Dallas_reset(int8_t gpio_pin_rx, int8_t gpio_pin_tx)
           // Presence condition ended
           presence_end = usec_passed;
         }
+      } else {
+        // Set the pin high so we have a clear starting level on the next write.
+        DIRECT_pinWrite(gpio_pin_tx, 1);
+        if (gpio_pin_rx == gpio_pin_tx) {
+          DIRECT_PINMODE_OUTPUT(gpio_pin_rx);
+        }
+        waiting_for_presence = false;
+        delayMicroseconds(4);
       }
       delayMicroseconds(2);
     }
@@ -809,6 +843,11 @@ uint8_t Dallas_search(uint8_t *newAddr, int8_t gpio_pin_rx, int8_t gpio_pin_tx)
           ROM_NO[rom_byte_number] &= ~rom_byte_mask;
         }
 
+        DIRECT_pinWrite(gpio_pin_tx, 1);
+        if (gpio_pin_rx == gpio_pin_tx) {
+          DIRECT_PINMODE_OUTPUT(gpio_pin_rx);
+        }
+
         // serial number search direction write bit
         Dallas_write_bit(search_direction, gpio_pin_rx, gpio_pin_tx);
 
@@ -885,9 +924,22 @@ void Dallas_write(uint8_t ByteToWrite, int8_t gpio_pin_rx, int8_t gpio_pin_tx)
 {
   uint8_t bitMask;
 
+  DIRECT_pinWrite(gpio_pin_tx, 1);
+  if (gpio_pin_rx == gpio_pin_tx) {
+    DIRECT_PINMODE_OUTPUT(gpio_pin_rx);
+  }
+#ifdef DEBUG_LOGIC_ANALYZER_PIN
+  // DEBUG code using logic analyzer for timings
+  DIRECT_pinWrite(DEBUG_LOGIC_ANALYZER_PIN, 1);
+#endif
   for (bitMask = 0x01; bitMask; bitMask <<= 1) {
     Dallas_write_bit((bitMask & ByteToWrite) ? 1 : 0, gpio_pin_rx, gpio_pin_tx);
   }
+#ifdef DEBUG_LOGIC_ANALYZER_PIN
+  // DEBUG code using logic analyzer for timings
+  DIRECT_pinWrite(DEBUG_LOGIC_ANALYZER_PIN, 0);
+#endif
+
 }
 
 /*********************************************************************************************\
@@ -898,7 +950,7 @@ uint8_t Dallas_read_bit(int8_t gpio_pin_rx, int8_t gpio_pin_tx)
   if (gpio_pin_rx == -1) { return 0; }
 
   if (gpio_pin_tx == -1) { return 0; }
-  const uint64_t start = getMicros64();
+  uint64_t start = 0;
   uint8_t r            = Dallas_read_bit_ISR(gpio_pin_rx, gpio_pin_tx, start);
 
   while (usecPassedSince(start) < 70ll) {
@@ -915,14 +967,14 @@ uint8_t Dallas_read_bit(int8_t gpio_pin_rx, int8_t gpio_pin_tx)
 uint8_t DALLAS_IRAM_ATTR Dallas_read_bit_ISR(
   int8_t        gpio_pin_rx,
   int8_t        gpio_pin_tx,
-  unsigned long start)
+  uint64_t& start)
 {
   uint8_t r;
 
   {
     noInterrupts();
+    start = getMicros64();
     DIRECT_pinWrite(gpio_pin_tx, 0);
-
     if (gpio_pin_rx == gpio_pin_tx) {
       DIRECT_PINMODE_OUTPUT(gpio_pin_rx);
     }
@@ -960,7 +1012,7 @@ void Dallas_write_bit(uint8_t v, int8_t gpio_pin_rx, int8_t gpio_pin_tx)
   // write 0: low 60 usec, high 10 usec
   const long low_time  = (v & 1) ? 6 : 60;
   const long high_time = (v & 1) ? 64 : 10;
-  uint64_t   start     = getMicros64();
+  uint64_t   start     = 0;
 
   Dallas_write_bit_ISR(v, gpio_pin_rx, gpio_pin_tx, low_time, high_time, start);
 
@@ -977,12 +1029,9 @@ void DALLAS_IRAM_ATTR Dallas_write_bit_ISR(uint8_t   v,
                                            uint64_t& start)
 {
   noInterrupts();
+  start     = getMicros64();
   DIRECT_pinWrite(gpio_pin_tx, 0);
 
-  if (gpio_pin_rx == gpio_pin_tx) {
-    DIRECT_PINMODE_OUTPUT(gpio_pin_rx);
-  }
-
   while (usecPassedSince(start) < low_time) {
     // output remains low
   }

src/src/Helpers/Dallas1WireHelper.h

@@ -173,7 +173,7 @@ void    Dallas_write(uint8_t ByteToWrite,
 *  See https://github.com/espressif/arduino-esp32/issues/1335
 \*********************************************************************************************/
 uint8_t Dallas_read_bit(int8_t gpio_pin_rx, int8_t gpio_pin_tx);
-uint8_t Dallas_read_bit_ISR(int8_t gpio_pin_rx, int8_t gpio_pin_tx, unsigned long start);
+uint8_t Dallas_read_bit_ISR(int8_t gpio_pin_rx, int8_t gpio_pin_tx, uint64_t& start);
 
 /*********************************************************************************************\
 *  Dallas Write bit

src/src/PluginStructs/P004_data_struct.cpp

@@ -5,6 +5,18 @@
 
 P004_data_struct::P004_data_struct(int8_t pin_rx, int8_t pin_tx, const uint8_t addr[], uint8_t res) : _gpio_rx(pin_rx), _gpio_tx(pin_tx), _res(res)
 {
+  // Explicitly set the pinMode using the "slow" pinMode function
+  // This way we know for sure the state of any pull-up or -down resistor is known.
+  pinMode(_gpio_rx, INPUT);
+
+  // The Shelly 1 temp. addon uses separate
+  // input and output pins, and therefore
+  // doesn't switch between input and output
+  // when running.
+  if (_gpio_rx != _gpio_tx) {
+    pinMode(_gpio_tx, OUTPUT);
+  }
+
   if ((_res < 9) || (_res > 12)) { _res = 12; }
 
   add_addr(addr, 0);