fix: review fixes for the ESP32-P4 build

- The pre-v3 build is for chips before v2.0, not v3.0: the esp32p4_es
  libraries stop at v1.99.  The CI label and comments say so, and that no
  build runs on v2.x or v3.00.
- A save refuses a clock mode the chip can't use: ProtoSettings takes
  CLKMode 0-ETH_CLOCK_ID_MAX, 0 only on the P4.  Before, 1-3 saved and
  then stopped Ethernet at the next start.  A range of one value reads
  "must be 0".
- Tests for the pin limit (54 taken, 55 refused) and the clock limits.
- The web UI's P4 pins leave out strapping pin 36 too.
- The preset says ESP32-P4-WIFI6-POE-ETH, the board's name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

.github/workflows/build.yaml

@@ -120,7 +120,7 @@ jobs:
             fsname: SomfyController.littlefs.esp32p4.bin
             obname: SomfyController.onboard.esp32p4.bin
           - env: esp32p4es
-            name: ESP32-P4 (before v3.0)
+            name: ESP32-P4 (before v2.0)
             chip: ESP32-P4
             addr_bootloader: "0x2000"
             addr_fs: "0x310000"

data-src/index.js

@@ -1579,7 +1579,7 @@ class Wifi {
     { val: 5, label: 'wESP32 v7+ - Silicognition', clk: 0, ct: 2, addr: 0, pwr: -1, mdc: 16, mdio: 17 },
     { val: 6, label: 'wESP32 < v7 - Silicognition', clk: 0, ct: 0, addr: 0, pwr: -1, mdc: 16, mdio: 17 },
     { val: 1, label: 'WT32-ETH01 - Wireless Tag', clk: 0, ct: 0, addr: 1, pwr: 16, mdc: 23, mdio: 18 },
-    { val: 8, label: 'ESP32-P4-ETH - Waveshare', clk: 0, ct: 1, addr: 1, pwr: 51, mdc: 31, mdio: 52, p4: true }
+    { val: 8, label: 'ESP32-P4-WIFI6-POE-ETH - Waveshare', clk: 0, ct: 1, addr: 1, pwr: 51, mdc: 31, mdio: 52, p4: true }
     ];
     ethClockModes = [{ val: 0, label: 'GPIO0 IN' }, { val: 1, label: 'GPIO0 OUT' }, { val: 2, label: 'GPIO16 OUT' }, { val: 3, label: 'GPIO17 OUT' }];
     // The P4 takes the clock in on GPIO 50 only (eth_clock_id_t in EthPhy.h).
@@ -2735,8 +2735,9 @@ class Somfy {
         { name: 'c5', maxPins: 28, inputs: [13, 14, 15, 16, 17, 18, 19, 20, 21, 22], outputs: [13, 14, 15, 16, 17, 18, 19, 20, 21, 22] },
         { name: 'c6', maxPins: 23, inputs: [], outputs: [] },
         // Without the C6's SDIO link (14-19, 54), USB Serial/JTAG (24, 25),
-        // Ethernet (28-31, 34, 35, 49-52) and UART0 (37, 38).
+        // Ethernet (28-31, 34, 35, 49-52), UART0 (37, 38) and the strapping
-        { name: 'p4', maxPins: 54, inputs: [14, 15, 16, 17, 18, 19, 24, 25, 28, 29, 30, 31, 34, 35, 37, 38, 49, 50, 51, 52, 54], outputs: [14, 15, 16, 17, 18, 19, 24, 25, 28, 29, 30, 31, 34, 35, 37, 38, 49, 50, 51, 52, 54] }
+        // pins (34-38).
+        { name: 'p4', maxPins: 54, inputs: [14, 15, 16, 17, 18, 19, 24, 25, 28, 29, 30, 31, 34, 35, 36, 37, 38, 49, 50, 51, 52, 54], outputs: [14, 15, 16, 17, 18, 19, 24, 25, 28, 29, 30, 31, 34, 35, 36, 37, 38, 49, 50, 51, 52, 54] }
     ];
 
     loadPins(type, sel, opt) {

docs/adr/0003-pioarduino-idf55.md

@@ -15,8 +15,8 @@ We moved every firmware env from `espressif32 @ 6.13.0` (Arduino 2.0.17 on ESP-I
 
   > **Amended by [ADR 0006](0006-two-security-modes-eleven-http-routes.md) (4.0.0).** The C3 builds again. Without the old API (ports 8080 and 8081), its image is 1,498,512 bytes at level 2 and leaves 72.6 KB of the OTA slot. It runs on chip revision v0.3 or later only: the framework's C3 libraries are built with `CONFIG_ESP32C3_REV_MIN_3`. Its default radio pins moved off GPIO 12-17, which hold the SPI flash.
 - **The C5 is updated over USB.** New in 4.0.0. At level 2 its image left 3.4 KB of the OTA slot, under CI's 8 KB, so like the C6 it is released as a debug build with one 3 MB app slot (`esp32_huge_app.csv`), which no OTA can write. The framework's C5 libraries run on chip revision v1.0 or later only (`CONFIG_ESP32C5_REV_MIN_FULL=100`). Its WiFi has 5 GHz too, so the passive scans for the configured network (`src/ESPNetwork.cpp`, 300 ms a channel) cover about 38 channels instead of 13: about 11 s instead of 4 s, depending on the country's channels.
-- **The P4 has two builds.** New in 4.0.0, for boards such as Waveshare's ESP32-P4-WIFI6-POE-ETH. The framework's P4 libraries run on chip revision v3.01 or later only (`CONFIG_ESP32P4_REV_MIN_FULL=301`), its `esp32p4_es` libraries on revisions before v2.0, so `esp32p4` (chip model `p4`) and `esp32p4es` (`p4es`) each get their own release files. Both keep the OTA slot: at level 2 their images leave about 128 KB. WiFi goes through the board's ESP32-C6 on SDIO (esp-hosted, the framework's pins 14-19 and reset 54), so the C6 has to run a matching esp-hosted slave firmware, which Waveshare's boards ship with.
+- **The P4 has two builds.** New in 4.0.0, for boards such as Waveshare's ESP32-P4-WIFI6-POE-ETH. The framework's P4 libraries run on chip revision v3.01 or later only (`CONFIG_ESP32P4_REV_MIN_FULL=301`), its `esp32p4_es` libraries on revisions before v2.0, so `esp32p4` (chip model `p4`) and `esp32p4es` (`p4es`) each get their own release files. No build runs on v2.x or v3.00. Both keep the OTA slot: at level 2 their images leave about 128 KB. WiFi goes through the board's ESP32-C6 on SDIO (esp-hosted, the framework's pins 14-19 and reset 54), so the C6 has to run a matching esp-hosted slave firmware, which Waveshare's boards ship with.
-- **Ethernet PHY ids are ours.** Arduino 3.x renumbered `eth_phy_type_t` (`ETH_PHY_GENERIC` is 0). NVS, backups and the web UI keep the Arduino 2.x numbers as `eth_phy_id_t` (`src/EthPhy.h`), translated by name when Ethernet starts. Never cast a stored id to the framework enum. The RMII clock mode is kept the same way, as `eth_clock_id_t` in the ESP32's numbering: the P4's `eth_clock_mode_t` is the IDF's `emac_rmii_clock_mode_t`, whose 0 is a deprecated default. On the P4 only clock in (GPIO 50) translates, since `ETH.begin()` keeps the clock on that pin.
+- **Ethernet PHY ids are ours.** Arduino 3.x renumbered `eth_phy_type_t` (`ETH_PHY_GENERIC` is 0). NVS, backups and the web UI keep the Arduino 2.x numbers as `eth_phy_id_t` (`src/EthPhy.h`), translated by name when Ethernet starts. Never cast a stored id to the framework enum. The RMII clock mode is kept the same way, as `eth_clock_id_t` in the ESP32's numbering: the P4's `eth_clock_mode_t` is the IDF's `emac_rmii_clock_mode_t`, whose 0 is a deprecated default. On the P4 only clock in (GPIO 50) translates, since `ETH.begin()` keeps the clock on that pin, and a save refuses the others (`ETH_CLOCK_ID_MAX`).
 - **Ethernet only on the original ESP32 and the P4.** Arduino 3.x has the RMII MAC API only where `CONFIG_ETH_USE_ESP32_EMAC` is set. S3, C3, C5 and C6 build without Ethernet.
 - **Updates roll back until proven.** We override the core's `verifyRollbackLater()` hook to return true, and `loop()` marks the new firmware valid once it has run 60 s and the network is up. A reset before then boots the previous firmware. The LittleFS image an update wrote stays, so the old firmware runs with the new web UI.
 - **Tooling.** pioarduino needs its own Core fork (`pip install pioarduino`, the pioarduino IDE extension in VS Code), which it installs into `~/.platformio/penv` in place of PlatformIO Core.

platformio.ini

@@ -130,6 +130,7 @@ build_flags =
 ; ESP32-C6 on SDIO (esp-hosted), Ethernet on the P4's own RMII MAC.  The
 ; framework's P4 libraries run on chip revision v3.01 or later only, and its
 ; esp32p4_es libraries on revisions before v2.0, so each gets its own build.
+; No build runs on v2.x or v3.00.
 [env:esp32p4]
 extends = esp32
 board = esp32-p4_r3

src/ConfigSettings.cpp

@@ -146,8 +146,8 @@ bool ConfigSettings::begin() {
       break;
     #endif
     #ifdef CONFIG_IDF_TARGET_ESP32P4
-    // Chips before v3.0 run their own build (esp32p4_es libraries), so they
+    // Chips before v2.0 run their own build (esp32p4_es libraries), so they
-    // get their own release files.
+    // get their own release files.  v2.x and v3.00 run neither.
     case esp_chip_model_t::CHIP_ESP32P4:
       #if CONFIG_ESP32P4_SELECTS_REV_LESS_V3
       strcpy(this->chipModel, "p4es");

src/EthPhy.h

@@ -35,9 +35,16 @@ enum class eth_clock_id_t : uint8_t {
   GPIO16_OUT = 2,
   GPIO17_OUT = 3
 };
+// The highest clock id the chip takes: the P4's ETH.begin() takes the clock
+// in on GPIO 50 only.
+#if defined(CONFIG_IDF_TARGET_ESP32P4)
+constexpr uint8_t ETH_CLOCK_ID_MAX = 0;
+#else
+constexpr uint8_t ETH_CLOCK_ID_MAX = 3;
+#endif
 
 // The GPIO a stored clock id puts the RMII clock on, -1 for an id the chip
-// can't use.  The P4's ETH.begin() takes the clock in on GPIO 50 only.
+// can't use.
 inline int8_t ethClockPin(uint8_t id) {
 #if defined(CONFIG_IDF_TARGET_ESP32P4)
   return static_cast<eth_clock_id_t>(id) == eth_clock_id_t::GPIO0_IN ? 50 : -1;

src/ProtoSettings.cpp

@@ -3,6 +3,7 @@
 #include <string.h>
 #include "ProtoSettings.h"
 #include "ProtoParse.h"
+#include "EthPhy.h"
 
 static const char *const SECTIONS[] = {"general", "network", "mqtt", "security", "radio"};
 const char *settingsSectionName(settings_section_t section) { return SECTIONS[static_cast<uint8_t>(section)]; }
@@ -45,7 +46,7 @@ static const settings_field_t FIELDS[] = {
   {settings_part_t::wifi, proto_field_t::hidden, nullptr, 0, 0},
   {settings_part_t::ethernet, proto_field_t::boardType, nullptr, 0, 255},
   {settings_part_t::ethernet, proto_field_t::phyType, nullptr, 0, 255},
-  {settings_part_t::ethernet, proto_field_t::CLKMode, nullptr, 0, 3},
+  {settings_part_t::ethernet, proto_field_t::CLKMode, nullptr, 0, ETH_CLOCK_ID_MAX},
   {settings_part_t::ethernet, proto_field_t::phyAddress, nullptr, -1, 31},
   // Pins run to GPIO 54, the P4's last.
   {settings_part_t::ethernet, proto_field_t::PWRPin, nullptr, -1, 54},
@@ -138,7 +139,8 @@ static const char *checkValue(const settings_field_t &s, const proto_field_info_
       return why;
     }
     if(inRange) return nullptr;
-    snprintf(why, sizeof(why), "%s must be %ld-%ld", f.name, (long)s.lo, (long)s.hi);
+    if(s.lo == s.hi) snprintf(why, sizeof(why), "%s must be %ld", f.name, (long)s.lo);
+    else snprintf(why, sizeof(why), "%s must be %ld-%ld", f.name, (long)s.lo, (long)s.hi);
     return why;
   }
   if(f.kind == 'n') {

test/test_ethphy/test_ethphy.cpp

@@ -77,6 +77,12 @@ static void test_rejects_unknown_clock_ids() {
   TEST_ASSERT_FALSE(ethClockMode(4, mode));
   TEST_ASSERT_FALSE(ethClockMode(255, mode));
 }
+static void test_a_save_takes_each_clock_id_that_translates() {
+  eth_clock_mode_t mode;
+  TEST_ASSERT_EQUAL_UINT8(3, ETH_CLOCK_ID_MAX);
+  TEST_ASSERT_TRUE(ethClockMode(ETH_CLOCK_ID_MAX, mode));
+  TEST_ASSERT_FALSE(ethClockMode(ETH_CLOCK_ID_MAX + 1, mode));
+}
 static void test_clock_pins() {
   TEST_ASSERT_EQUAL_INT8(0, ethClockPin(0));
   TEST_ASSERT_EQUAL_INT8(0, ethClockPin(1));
@@ -95,6 +101,7 @@ int main(int, char **) {
   RUN_TEST(test_clock_ids_match_stored_values);
   RUN_TEST(test_translates_each_clock_mode);
   RUN_TEST(test_rejects_unknown_clock_ids);
+  RUN_TEST(test_a_save_takes_each_clock_id_that_translates);
   RUN_TEST(test_clock_pins);
   return UNITY_END();
 }

test/test_ethphy_p4/test_ethphy_p4.cpp

@@ -32,6 +32,14 @@ static void test_rejects_clock_out() {
   TEST_ASSERT_FALSE(ethClockMode(3, mode));
   TEST_ASSERT_FALSE(ethClockMode(4, mode));
 }
+static void test_a_save_takes_clock_in_only() {
+  // ProtoSettings refuses a CLKMode over this, so a save can't store a mode
+  // that stops Ethernet at the next start.
+  eth_clock_mode_t mode;
+  TEST_ASSERT_EQUAL_UINT8(0, ETH_CLOCK_ID_MAX);
+  TEST_ASSERT_TRUE(ethClockMode(ETH_CLOCK_ID_MAX, mode));
+  TEST_ASSERT_FALSE(ethClockMode(ETH_CLOCK_ID_MAX + 1, mode));
+}
 static void test_clock_pin_is_50() {
   TEST_ASSERT_EQUAL_INT8(50, ethClockPin(0));
   TEST_ASSERT_EQUAL_INT8(-1, ethClockPin(1));
@@ -44,6 +52,7 @@ int main(int, char **) {
   RUN_TEST(test_ip101_is_the_tlk110_id);
   RUN_TEST(test_clock_in_is_ext_in);
   RUN_TEST(test_rejects_clock_out);
+  RUN_TEST(test_a_save_takes_clock_in_only);
   RUN_TEST(test_clock_pin_is_50);
   return UNITY_END();
 }

test/test_protosettings/test_protosettings.cpp

@@ -98,6 +98,13 @@ static void test_whole_numbers_are_checked_against_what_the_setting_holds() {
   TEST_ASSERT_EQUAL_STRING("port must be 1-65535", expand(R"({"se":"mqtt","po":18446744073709551615})"));
   // A pin of -1 is none.
   TEST_ASSERT_GOOD(expand(R"({"se":"network","pw":-1})"));
+  // Pins run to GPIO 54, the P4's last.
+  TEST_ASSERT_GOOD(expand(R"({"se":"radio","tx":54})"));
+  TEST_ASSERT_EQUAL_STRING("TXPin must be 0-54", expand(R"({"se":"radio","tx":55})"));
+  TEST_ASSERT_GOOD(expand(R"({"se":"network","mi":54})"));
+  // The ESP32's four clock modes; a P4 build takes 0 only (ETH_CLOCK_ID_MAX).
+  TEST_ASSERT_GOOD(expand(R"({"se":"network","ck":3})"));
+  TEST_ASSERT_EQUAL_STRING("CLKMode must be 0-3", expand(R"({"se":"network","ck":4})"));
   TEST_ASSERT_GOOD(expand(R"({"se":"radio","tp":-30})"));
 }
 static void test_the_bit_length_is_56_or_80() {