Merge pull request #122 from cjkas/fix/gpio12-strapping

fix: warn of radio pins that can stop the board booting (#34)

data-src/index.html

@@ -919,20 +919,21 @@
                                 <label for="selTransMOSIPin">MOSI</label>
                             </div>
                             <div class="field-group radioPins">
-                                <select id="selTransMISOPin" name="transMISO" data-bind="transceiver.config.MISOPin" data-datatype="int"></select>
+                                <select id="selTransMISOPin" name="transMISO" data-bind="transceiver.config.MISOPin" data-datatype="int" onchange="somfy.checkBootPins();"></select>
                                 <label for="selTransMISOPin">MISO</label>
                             </div>
                         </div>
                         <div class="field-group1" style="margin-top:-20px;">
                             <div class="field-group radioPins">
-                                <select id="selTransTXPin" name="transTX" data-bind="transceiver.config.TXPin" data-datatype="int"></select>
+                                <select id="selTransTXPin" name="transTX" data-bind="transceiver.config.TXPin" data-datatype="int" onchange="somfy.checkBootPins();"></select>
                                 <label for="selTransTXPin">TX</label>
                             </div>
                             <div class="field-group radioPins">
-                                <select id="selTransRXPin" name="transRX" data-bind="transceiver.config.RXPin" data-datatype="int"></select>
+                                <select id="selTransRXPin" name="transRX" data-bind="transceiver.config.RXPin" data-datatype="int" onchange="somfy.checkBootPins();"></select>
                                 <label for="selTransRXPin">RX</label>
                             </div>
                         </div>
+                        <div id="divRadioBootPins" style="display:none;color:#e65100;font-size:12px;margin:-14px 0 14px 0;white-space:normal;"></div>
                         <div class="field-group" style="display:inline-block;width:auto;min-width:247px;margin-top:-20px;vertical-align:top;">
                             <div class="field-group">
                                 <input id="slidFrequency" name="frequency" type="range" min="433000" max="434399" step="1" style="width:100%;" data-bind="transceiver.config.frequency" data-datatype="float" data-mult="1000" oninput="somfy.frequencyChanged(this);" />

data-src/index.js

@@ -418,6 +418,9 @@ function sendEdit(name, fields, target, cb) {
 // A command's repeat count, when the caller gave one.
 function repeatOf(repeat) { return typeof repeat === 'number' ? repeat : undefined; }
 function gone(what) { ui.errorMessage(`The ${what} is no longer there.`); }
+// The chip model the header sent: '' for the original ESP32, then s3, p4es
+// and so on.
+function chipModel() { return document.getElementById('divContainer').getAttribute('data-chipmodel') || ''; }
 // The lists redraw from the model after events, a burst of them at once.
 // While the socket is down they wait for the next sync.
 var renderPending = {};
@@ -1601,7 +1604,7 @@ class Wifi {
         this.onETHBoardTypeChanged(document.getElementById('selETHBoardType'));
         this.initialized = true;
     }
-    isP4() { return (document.getElementById('divContainer').getAttribute('data-chipmodel') || '').startsWith('p4'); }
+    isP4() { return chipModel().startsWith('p4'); }
     clockModes() { return this.isP4() ? this.ethClockModesP4 : this.ethClockModes; }
     // The board presets, clock modes and pins of the chip: the ESP32's or the
     // P4's.  The chip comes with the sync, after init().
@@ -1944,8 +1947,9 @@ class Somfy {
         this.loadPins('input', document.getElementById('selTransRXPin'));
         //this.loadSomfy();
         ui.toElement(document.getElementById('divTransceiverSettings'), {
-            transceiver: { config: { proto: 0, SCKPin: 18, CSNPin: 5, MOSIPin: 23, MISOPin: 19, TXPin: 12, RXPin: 13, frequency: 433.42, rxBandwidth: 97.96, type: 56, deviation: 11.43, txPower: 10, enabled: false, noiseDetection: false } }
+            transceiver: { config: { proto: 0, SCKPin: 18, CSNPin: 5, MOSIPin: 23, MISOPin: 19, TXPin: 13, RXPin: 35, frequency: 433.42, rxBandwidth: 97.96, type: 56, deviation: 11.43, txPower: 10, enabled: false, noiseDetection: false } }
         });
+        this.checkBootPins();
         this.loadPins('out', document.getElementById('selShadeGPIOUp'));
         this.loadPins('out', document.getElementById('selShadeGPIODown'));
         this.loadPins('out', document.getElementById('selShadeGPIOMy'));
@@ -1969,8 +1973,17 @@ class Somfy {
             let config = ProtoViews.radio(proto.schema, radio);
             ui.toElement(document.getElementById('divTransceiverSettings'), { transceiver: { config: config } });
             document.getElementById('divRadioError').style.display = config.radioInit ? 'none' : '';
+            this.checkBootPins();
         });
     }
+    // Warns of radio pins that can stop the board booting (issue #34).
+    checkBootPins() {
+        let trans = ui.fromElement(document.getElementById('divTransceiverSettings')).transceiver;
+        let warnings = BootPins.radioWarnings(chipModel(), trans.config);
+        let div = document.getElementById('divRadioBootPins');
+        div.innerText = warnings.join('\n');
+        div.style.display = warnings.length > 0 ? '' : 'none';
+    }
     saveRadio() {
         let valid = true;
         let getIntValue = (fld) => { return parseInt(document.getElementById(fld).value, 10); };
@@ -2742,7 +2755,7 @@ class Somfy {
 
     loadPins(type, sel, opt) {
         while (sel.firstChild) sel.removeChild(sel.firstChild);
-        let cm = document.getElementById('divContainer').getAttribute('data-chipmodel');
+        let cm = chipModel();
         // The pre-v3 P4 build has the same pins.
         if (cm === 'p4es') cm = 'p4';
         let pm = this.pinMaps.find(x => x.name === cm) || { name: '', maxPins: 39, inputs: [], outputs: [] };
@@ -4189,7 +4202,7 @@ class Firmware {
         div.style.display = '';
     }
     updateFirmware() {
-        let chip = document.getElementById('divContainer').getAttribute('data-chipmodel') || '';
+        let chip = chipModel();
         // Firmware from before the field leaves it out: undefined, not false.
         if (proto.model.header.hasOtaSlot === false) {
             ui.infoMessage(`This device has no OTA slot, so its firmware is flashed over USB with SomfyController.onboard.esp32${chip}.bin, from the release's SomfyController.onboard.esp32${chip}.bin.zip. That image holds the web UI and an empty configuration too, so take a backup first and restore it afterwards.`);
@@ -4212,7 +4225,7 @@ class Firmware {
         let div = this.createFileUploader('/updateApplication');
         general.reloadApp = true;
         let inst = div.querySelector('div[id=divInstText]');
-        let chip = document.getElementById('divContainer').getAttribute('data-chipmodel') || '';
+        let chip = chipModel();
         inst.innerHTML = `<div style="font-size:14px;">Select a binary file [SomfyController.littlefs.esp32${chip}.bin] containing the littlefs data for the application then press the Upload File button.</div>`;
         if (this.isMobile()) {
             inst.innerHTML += `<div style="width:100%;color:red;text-align:center;font-weight:bold;"><span style="margin-top:7px;width:100%;background:yellow;padding:3px;display:inline-block;border-radius:5px;background:white;">WARNING<span></div>`;

data-src/js/pins.js

@@ -0,0 +1,43 @@
+// The strapping pins whose level at reset can keep each chip from starting
+// the firmware, or from being flashed over USB, as src/BootPins.h has them
+// for the log.  A radio that drives one at reset stops the board booting
+// until EN is pressed (issue #34), or esptool from connecting.  Keep the
+// pins, the radio pins checked and the shared RX/TX rule in step with that
+// file.
+// The minify step joins this file into index.js; Node loads it on its own for
+// the tests (test/js).
+class BootPins {
+    // A warning for each of the radio config's pins that can stop the board
+    // booting or being flashed.
+    static radioWarnings(chipModel, config) {
+        const chip = BootPins.CHIPS[chipModel || ''] || {};
+        const warnings = [];
+        for (const [field, name] of BootPins.RADIO_PINS) {
+            const pin = config[field];
+            if (field === 'RXPin' && pin === config.TXPin) continue;
+            const effect = chip[pin];
+            if (effect) warnings.push(`${name} is on GPIO-${pin}, a strapping pin the radio can drive at reset.  ${effect}`);
+        }
+        return warnings;
+    }
+}
+BootPins.DOWNLOAD = 'Held low, it starts the download mode instead of the firmware: move it to another pin.';
+BootPins.FLASH_1V8 = 'Held high, it powers the flash at 1.8 V and the board bootloops: move it to another pin.';
+BootPins.NO_USB_HIGH = 'Held high, it stops flashing over USB, so disconnect it to flash.';
+BootPins.NO_USB_LOW = 'Held low, it stops flashing over USB, so disconnect it to flash.';
+// By the chip model the header sends, '' for the original ESP32.
+BootPins.CHIPS = {
+    '': { 0: BootPins.DOWNLOAD, 12: BootPins.FLASH_1V8, 2: BootPins.NO_USB_HIGH },
+    s2: { 0: BootPins.DOWNLOAD, 45: BootPins.FLASH_1V8, 46: BootPins.NO_USB_HIGH },
+    s3: { 0: BootPins.DOWNLOAD, 45: BootPins.FLASH_1V8, 46: BootPins.NO_USB_HIGH },
+    c3: { 9: BootPins.DOWNLOAD, 8: BootPins.NO_USB_LOW },
+    c5: { 28: BootPins.DOWNLOAD },
+    c6: { 9: BootPins.DOWNLOAD, 8: BootPins.NO_USB_LOW },
+    p4: { 35: BootPins.DOWNLOAD },
+    p4es: { 35: BootPins.DOWNLOAD }
+};
+// The radio pins the CC1101 drives: at power-up GDO0 puts out a clock, GDO2
+// CHIP_RDYn, and SO follows CHIP_RDYn while CSN is low.  Once it is set up,
+// GDO2 puts out what it hears.
+BootPins.RADIO_PINS = [['TXPin', 'TX'], ['RXPin', 'RX'], ['MISOPin', 'MISO']];
+if (typeof module !== 'undefined' && module.exports) module.exports = { BootPins };

minify.py

@@ -58,7 +58,7 @@ PNG_EXTENSIONS = {".png"}
 # comes before index.js, which uses it.  A source listed here isn't copied on
 # its own.
 BUNDLES = {
-    "index.js": ["js/schema.js", "js/model.js", "js/client.js", "js/views.js", "index.js"],
+    "index.js": ["js/schema.js", "js/model.js", "js/client.js", "js/views.js", "js/pins.js", "index.js"],
     "app.css": ["main.css", "widgets.css", "icons.css"],
 }
 

src/BootPins.h

@@ -0,0 +1,43 @@
+#ifndef BOOT_PINS_H
+#define BOOT_PINS_H
+#include <stdint.h>
+#if __has_include(<sdkconfig.h>)
+#include <sdkconfig.h>
+#endif
+
+// The strapping pins whose level at reset can keep the chip from starting
+// the firmware, or from being flashed over USB.  The ROM reads them before
+// any code runs, so a radio that drives one at reset stops the board booting
+// until EN is pressed (issue #34), or esptool from connecting.  The strapping
+// pins that only quiet the boot log or set the SDIO timing aren't listed.
+// The web UI has the same pins in data-src/js/pins.js, and checks the same
+// radio pins as transceiver_config_t::apply(): keep the three in step.
+constexpr const char *BOOT_PIN_DOWNLOAD = "Held low, it starts the download mode instead of the firmware: move it to another pin.";
+constexpr const char *BOOT_PIN_FLASH_1V8 = "Held high, it powers the flash at 1.8 V and the board bootloops: move it to another pin.";
+constexpr const char *BOOT_PIN_NO_USB_HIGH = "Held high, it stops flashing over USB, so disconnect it to flash.";
+constexpr const char *BOOT_PIN_NO_USB_LOW = "Held low, it stops flashing over USB, so disconnect it to flash.";
+
+// Returns what the pin does at reset and what to do, or nullptr for a pin
+// that is safe.
+inline const char *bootPinEffect(uint8_t pin) {
+#if defined(CONFIG_IDF_TARGET_ESP32S2) || defined(CONFIG_IDF_TARGET_ESP32S3)
+  if(pin == 0) return BOOT_PIN_DOWNLOAD;
+  if(pin == 45) return BOOT_PIN_FLASH_1V8;
+  if(pin == 46) return BOOT_PIN_NO_USB_HIGH;
+#elif defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32C6)
+  if(pin == 9) return BOOT_PIN_DOWNLOAD;
+  if(pin == 8) return BOOT_PIN_NO_USB_LOW;
+#elif defined(CONFIG_IDF_TARGET_ESP32C5)
+  if(pin == 28) return BOOT_PIN_DOWNLOAD;
+#elif defined(CONFIG_IDF_TARGET_ESP32P4)
+  if(pin == 35) return BOOT_PIN_DOWNLOAD;
+#else
+  // The original ESP32.  MTDI picks the flash voltage unless the
+  // VDD_SDIO efuses are burned.
+  if(pin == 0) return BOOT_PIN_DOWNLOAD;
+  if(pin == 12) return BOOT_PIN_FLASH_1V8;
+  if(pin == 2) return BOOT_PIN_NO_USB_HIGH;
+#endif
+  return nullptr;
+}
+#endif

src/ConfigRecords.h

@@ -183,7 +183,7 @@ struct trans_record_t {
   uint8_t MOSIPin = 23;
   uint8_t MISOPin = 19;
   uint8_t TXPin = 13;
-  uint8_t RXPin = 12;
+  uint8_t RXPin = 35;
   float frequency = 433.42;
   float rxBandwidth = 99.97;
   float deviation = 47.60;

src/Transceiver.cpp

@@ -8,6 +8,7 @@
 #include "driver/gpio.h"
 #include "esp_log.h"
 #include "Utils.h"
+#include "BootPins.h"
 #include "ConfigSettings.h"
 #include "Somfy.h"
 #include "outputs/ProtoSocket.h"
@@ -523,8 +524,11 @@ void transceiver_config_t::load() {
         break;
 #endif
       default:
+        // RX was on 12, a strapping pin (issue #34).  A board that saved its
+        // radio settings keeps its pins.  35 is input only, which RX is; 36
+        // and 39 take false interrupts.
         this->TXPin = 13;
-        this->RXPin = 12;
+        this->RXPin = 35;
         this->MOSIPin = 23;
         this->MISOPin = 19;
         this->SCKPin = 18;
@@ -571,6 +575,10 @@ void transceiver_config_t::load() {
     pref.end();
     //this->printBuffer = somfy.transceiver.printBuffer;
 }
+static void warnBootPin(const char *name, uint8_t pin) {
+  const char *effect = bootPinEffect(pin);
+  if(effect) ESP_LOGW(TAG, "Radio %s is on GPIO%u, a strapping pin the radio can drive at reset.  %s", name, pin, effect);
+}
 void transceiver_config_t::apply() {
     Preferences pref;
     // A transmission in flight stops after its current frame so the radio can
@@ -595,6 +603,13 @@ void transceiver_config_t::apply() {
       pref.end();
       if(!radioInit) return;
       ESP_LOGD(TAG, "Applying radio settings - Setting Data Pins RX:%u TX:%u", this->RXPin, this->TXPin);
+      // The pins the CC1101 drives: at power-up GDO0 puts out a clock, GDO2
+      // CHIP_RDYn, and SO follows CHIP_RDYn while CSN is low.  Once it is
+      // set up, GDO2 puts out what it hears.  The web UI checks the same
+      // (data-src/js/pins.js).
+      warnBootPin("TX", this->TXPin);
+      if(this->RXPin != this->TXPin) warnBootPin("RX", this->RXPin);
+      warnBootPin("MISO", this->MISOPin);
       //if(this->TXPin != this->RXPin)
       //  pinMode(this->TXPin, OUTPUT);
       //pinMode(this->RXPin, INPUT);

src/Transceiver.h

@@ -30,7 +30,7 @@ struct transceiver_config_t {
     radio_proto proto = radio_proto::RTS;
     uint8_t SCKPin = 18;
     uint8_t TXPin = 13;
-    uint8_t RXPin = 12;
+    uint8_t RXPin = 35;
     uint8_t MOSIPin = 23;
     uint8_t MISOPin = 19;
     uint8_t CSNPin = 5;

test/js/pins.test.js

@@ -0,0 +1,60 @@
+// The radio pins that can stop the board booting (data-src/js/pins.js).
+const test = require('node:test');
+const assert = require('node:assert/strict');
+const { BootPins } = require('../../data-src/js/pins.js');
+
+// transceiver_config_t::load()'s defaults for the original ESP32.
+const esp32Defaults = { SCKPin: 18, CSNPin: 5, MOSIPin: 23, MISOPin: 19, TXPin: 13, RXPin: 35 };
+
+test('the original ESP32 defaults are safe', () => {
+    assert.deepEqual(BootPins.radioWarnings('', esp32Defaults), []);
+});
+
+test('RX on GPIO12 warns of the flash voltage on the original ESP32', () => {
+    // Issue #34.
+    const warnings = BootPins.radioWarnings('', Object.assign({}, esp32Defaults, { RXPin: 12 }));
+    assert.equal(warnings.length, 1);
+    assert.match(warnings[0], /^RX is on GPIO-12/);
+    assert.match(warnings[0], /1\.8 V/);
+});
+
+test('TX on GPIO2 warns of flashing over USB on the original ESP32', () => {
+    // The WT32-ETH01 wiring in the wiki.
+    const warnings = BootPins.radioWarnings('', Object.assign({}, esp32Defaults, { TXPin: 2 }));
+    assert.equal(warnings.length, 1);
+    assert.match(warnings[0], /^TX is on GPIO-2, .*Held high, it stops flashing over USB, so disconnect it to flash\.$/);
+});
+
+test('GPIO8 held low blocks flashing on the C3 and C6', () => {
+    const config = { SCKPin: 4, CSNPin: 7, MOSIPin: 6, MISOPin: 5, TXPin: 10, RXPin: 8 };
+    for (const chip of ['c3', 'c6']) {
+        const warnings = BootPins.radioWarnings(chip, config);
+        assert.equal(warnings.length, 1);
+        assert.match(warnings[0], /^RX is on GPIO-8, .*Held low, it stops flashing over USB/);
+    }
+});
+
+test('an unknown chip model warns of nothing', () => {
+    assert.deepEqual(BootPins.radioWarnings('h2', Object.assign({}, esp32Defaults, { RXPin: 12 })), []);
+});
+
+test('a missing chip model is the original ESP32', () => {
+    assert.equal(BootPins.radioWarnings(null, Object.assign({}, esp32Defaults, { TXPin: 0 })).length, 1);
+});
+
+test('GPIO12 is safe on the S3', () => {
+    assert.deepEqual(BootPins.radioWarnings('s3', { SCKPin: 12, CSNPin: 10, MOSIPin: 11, MISOPin: 13, TXPin: 15, RXPin: 14 }), []);
+});
+
+test('MISO and TX warn, the pins the radio only reads do not', () => {
+    const config = { SCKPin: 4, CSNPin: 0, MOSIPin: 1, MISOPin: 9, TXPin: 10, RXPin: 3 };
+    assert.deepEqual(BootPins.radioWarnings('c3', config).map((w) => w.split(' ')[0]), ['MISO']);
+    assert.deepEqual(BootPins.radioWarnings('c6', Object.assign({}, config, { MISOPin: 2, TXPin: 9, RXPin: 9 })).map((w) => w.split(' ')[0]),
+        ['TX']);
+});
+
+test('the pre-v3 P4 has the P4 pins', () => {
+    const p4 = { SCKPin: 27, CSNPin: 46, MOSIPin: 32, MISOPin: 33, TXPin: 2, RXPin: 35 };
+    assert.equal(BootPins.radioWarnings('p4', p4).length, 1);
+    assert.deepEqual(BootPins.radioWarnings('p4es', p4), BootPins.radioWarnings('p4', p4));
+});

test/test_bootpins/test_bootpins.cpp

@@ -0,0 +1,60 @@
+#include <unity.h>
+#include <string.h>
+// No target defined: the original ESP32.
+#include "BootPins.h"
+
+void setUp() {}
+void tearDown() {}
+
+static void test_gpio12_picks_the_flash_voltage() {
+  // Issue #34: a CC1101 holding GDO2 high on GPIO12 at power-up.
+  const char *effect = bootPinEffect(12);
+  TEST_ASSERT_NOT_NULL(effect);
+  TEST_ASSERT_NOT_NULL(strstr(effect, "1.8 V"));
+}
+
+static void test_gpio0_picks_the_boot_mode() {
+  const char *effect = bootPinEffect(0);
+  TEST_ASSERT_NOT_NULL(effect);
+  TEST_ASSERT_NOT_NULL(strstr(effect, "download mode"));
+}
+
+static void test_gpio2_blocks_flashing_over_usb() {
+  // It only counts with GPIO0 low, so the board boots, but esptool can't
+  // start the download mode while it is high.
+  const char *effect = bootPinEffect(2);
+  TEST_ASSERT_NOT_NULL(effect);
+  TEST_ASSERT_NOT_NULL(strstr(effect, "high"));
+  TEST_ASSERT_NOT_NULL(strstr(effect, "USB"));
+}
+
+static void test_harmless_strapping_pins_are_not_listed() {
+  // 5 and 15 set the SDIO timing and quiet the boot log.  5 is the default
+  // CSN.
+  TEST_ASSERT_NULL(bootPinEffect(5));
+  TEST_ASSERT_NULL(bootPinEffect(15));
+}
+
+static void test_default_radio_pins_are_safe() {
+  // transceiver_config_t::load()'s defaults for the original ESP32.
+  const uint8_t pins[] = {13, 35, 23, 19, 18, 5};
+  for(uint8_t pin : pins) TEST_ASSERT_NULL_MESSAGE(bootPinEffect(pin), "default radio pin");
+}
+
+static void test_other_chips_boot_pins_are_safe_here() {
+  TEST_ASSERT_NULL(bootPinEffect(8));
+  TEST_ASSERT_NULL(bootPinEffect(9));
+  TEST_ASSERT_NULL(bootPinEffect(45));
+  TEST_ASSERT_NULL(bootPinEffect(46));
+}
+
+int main(int, char **) {
+  UNITY_BEGIN();
+  RUN_TEST(test_gpio12_picks_the_flash_voltage);
+  RUN_TEST(test_gpio0_picks_the_boot_mode);
+  RUN_TEST(test_gpio2_blocks_flashing_over_usb);
+  RUN_TEST(test_harmless_strapping_pins_are_not_listed);
+  RUN_TEST(test_default_radio_pins_are_safe);
+  RUN_TEST(test_other_chips_boot_pins_are_safe_here);
+  return UNITY_END();
+}

test/test_bootpins_s3/test_bootpins_s3.cpp

@@ -0,0 +1,45 @@
+#include <unity.h>
+#include <string.h>
+// As on the ESP32-S3, where GPIO45 picks the flash voltage.
+#define CONFIG_IDF_TARGET_ESP32S3 1
+#include "BootPins.h"
+
+void setUp() {}
+void tearDown() {}
+
+static void test_gpio45_picks_the_flash_voltage() {
+  const char *effect = bootPinEffect(45);
+  TEST_ASSERT_NOT_NULL(effect);
+  TEST_ASSERT_NOT_NULL(strstr(effect, "1.8 V"));
+}
+
+static void test_gpio0_picks_the_boot_mode() {
+  TEST_ASSERT_NOT_NULL(bootPinEffect(0));
+}
+
+static void test_gpio46_blocks_flashing_over_usb() {
+  const char *effect = bootPinEffect(46);
+  TEST_ASSERT_NOT_NULL(effect);
+  TEST_ASSERT_NOT_NULL(strstr(effect, "USB"));
+}
+
+static void test_gpio12_is_safe() {
+  // 12 is the S3's default SCK.
+  TEST_ASSERT_NULL(bootPinEffect(12));
+}
+
+static void test_default_radio_pins_are_safe() {
+  // transceiver_config_t::load()'s defaults for the S3.
+  const uint8_t pins[] = {15, 14, 11, 13, 12, 10};
+  for(uint8_t pin : pins) TEST_ASSERT_NULL_MESSAGE(bootPinEffect(pin), "default radio pin");
+}
+
+int main(int, char **) {
+  UNITY_BEGIN();
+  RUN_TEST(test_gpio45_picks_the_flash_voltage);
+  RUN_TEST(test_gpio0_picks_the_boot_mode);
+  RUN_TEST(test_gpio46_blocks_flashing_over_usb);
+  RUN_TEST(test_gpio12_is_safe);
+  RUN_TEST(test_default_radio_pins_are_safe);
+  return UNITY_END();
+}