Rework Original into an HTTPS webhook tally counter - #1
Merged
Conversation
Turns the Home Buttons Original (A1) into a two-channel counter that POSTs each press to an HTTPS webhook instead of publishing over MQTT. Behaviour: a press blinks the LED, increments a locally-held counter, redraws the display and posts the absolute count. The device is authoritative, so a dropped request repairs itself on the next press and no retry queue is needed; seq is carried purely so the receiver can dedupe a retry and avoid notifying twice. After the first press the Wi-Fi association and TLS session stay open for a short window so a burst of presses shares one handshake. Removed: MQTT client and Home Assistant discovery, the temperature and humidity sensor, the runtime MDI icon downloader, the factory self-test, and the mini/pro/industrial variants. The pro targets had not compiled for some time - App has no bsl_input_ member for that variant, and touch.cpp emits enum values that do not exist. Icons are now generated by tools/make_icons.py and flashed as a SPIFFS image rather than downloaded at runtime. TLS now uses the ESP-IDF certificate bundle. The pinned root was the DST-cross-signed ISRG Root X1, which expired 2024-09-30 while the comment claimed 2030, and a custom icon server was passed a null CA - HTTPS with no verification at all. The backend sits behind Cloudflare, which rotates edge issuers, so a bundle is the only durable option. SPIFFS shrinks from 0x160000 to 0xA0000 and the app slots grow to 0x1A0000. The baseline build was at 95.8% of its app partition with ~55 kB spare; this one sits at 69.3% including the 14.7 kB cert bundle. Also fixed, all present upstream at v2.6.1: - wake-pin decode used log(mask)/log(2) on floats, which returns a nonsense pin when two buttons are held and is undefined for a zero mask - the network connect timeout compared absolute millis() to the timeout - _nvs_2_efuse passed eFuse bit counts as buffer lengths into char[9] - the text-shrink loops could spin forever below one glyph width - _efuse_burned declared uint8_t buf[8] and read 8 bits - task names were passed as a printf format string - HardwareDefinition had no default member initialisers - the IP string buffer was read without being zero-initialised - load_persisted opened NVS read-write on every boot - the setup AP password was "password123" on every unit shipped CI builds the firmware for the first time; upstream only deployed docs, which is how the pro targets drifted into not compiling unnoticed. Co-Authored-By: Claude <noreply@anthropic.com>
The partition layout changed in this fork, so a device coming from stock firmware has to be flashed over USB with otadata reset - otherwise the bootloader can follow a stale ota_1 pointer into what is now a different offset. The artifact bundle was missing that file, which made it insufficient for a clean flash. Ship partitions.csv alongside it as a record of the layout each build was made for. Co-Authored-By: Claude <noreply@anthropic.com>
upload-artifact derives the archive layout from the common ancestor of the paths it is given, so listing the images by their build-tree location buried them under .pio/build/original_release/ in the download - four levels deep before you reach a file you can flash. Stage them into a single directory instead so unzipping the artifact gives the five images and partitions.csv side by side. Co-Authored-By: Claude <noreply@anthropic.com>
The AP SSID is "HB-<random_id>" and is broadcast, so deriving the password from the same random id made it readable over the air by anyone in range - no better than the hardcoded "password123" it replaced, just less obvious. Use the serial number instead. It does not appear in the SSID, and it is recoverable from the device itself via the Device Info screen, which shows the unique id as HBTNS-<serial>-<random_id>. Also adds docs/counter-guide.md covering flashing offsets, the setup portal, the n8n receiver, troubleshooting and the compile-time tunables, and points the README at it. The rest of docs/ is inherited from upstream and documents the MQTT firmware, which no longer applies here - called out in both places rather than silently left to mislead. Co-Authored-By: Claude <noreply@anthropic.com>
Logs now come out of the USB-C connector in original_debug, so reading them no longer requires wiring a serial adapter to the CMSIS-DAP header. Debug build only. Release stays on UART0 on purpose: UART works from the first ROM bootloader byte, survives deep sleep, and lets the device run on battery while you read it. USB CDC does none of those, and release is the build that runs untethered. ESP32-S2 has no USB_SERIAL_JTAG peripheral, so this goes through the USB OTG controller and TinyUSB. Deep sleep tears the USB device down, which would make the port drop on every wake. Awake Mode fixes that, but with MQTT removed there was no way left to reach user_awake_mode - upstream set it over MQTT, so the flag was read on every boot and written by nothing. It is now a portal field, which both makes USB logging usable and un-orphans the flag. Verified in the generated sdkconfig for each env: original_release CONFIG_ESP_CONSOLE_UART_DEFAULT=y original_debug CONFIG_ESP_CONSOLE_USB_CDC=y Co-Authored-By: Claude <noreply@anthropic.com>
The LED latched on at the first press and stayed lit for the whole awake session, so it read as "device is awake" rather than "press not yet delivered". Rewire it around the webhook result instead: solid press accepted, POST in flight dark delivered (2xx) 3 fast blinks send failed - counter moved locally, notification did not 2 fast blinks button is not bound to a counter Presses no longer blink from the state handlers; _handle_counter_press owns lighting the LED and _flush_pending owns clearing it, so there is one place that decides what the light means. Tracking this per press was not enough. Pressing a button again while its first press was still in flight lit the LED, and then the first press's 200 cleared it while the second was still queued - dark with a press pending. The LED is per button, so the state has to be too: inflight_ counts queued -but-unconfirmed presses per button and the LED is only released when that returns to zero. It is incremented on the UI task and decremented on the main task, hence atomic. send_failed_ carries a failure across the rest of a burst so the button ends on the error pattern rather than just going out. Co-Authored-By: Claude <noreply@anthropic.com>
The buttons are physically two columns of three, so treat each column as a
whole counter read top to bottom rather than pairing them across rows:
1 2 title your label or icon, no action
3 4 count shows the running total, press to add one
5 6 minus press to correct
draw_main() already places even label indices on the left and odd on the
right in three rows, so label N lands next to button N with no display
changes needed.
The number stands alone on the middle row now - the title above says what
it counts, so the label no longer needs an "A "/"B " prefix, which frees
the whole cell for a legible figure. Only that middle row is owned by the
firmware; titles and minus glyphs stay exactly as configured.
Button mapping moves from four scalar constants to BTN_COUNTER_TITLE /
_INC / _DEC arrays indexed by counter, so _btn_to_counter is a loop and
adding a third counter would not need new branches.
Defaults are now A/B for the titles and mdi:minus for the decrements -
minus.bmp already ships in the SPIFFS image.
Co-Authored-By: Claude <noreply@anthropic.com>
USB CDC logging landed in original_debug, but the workflow only ever ran original_release - so the build you need in order to read logs over the USB-C cable was not being produced, let alone published. Build both environments and publish them as two artifacts. They are not interchangeable: original_debug routes the console to USB CDC and uses QIO flash mode, so its bootloader.bin differs from the release one as well as its firmware.bin (27408 vs 20048 bytes). Two artifacts rather than one with subdirectories, because both environments produce a firmware.bin and a bootloader.bin - a single flat directory cannot hold both, and subdirectories would reintroduce exactly the nesting the flattening removed. The headroom check now covers both and reports each on the step summary, rather than passing on release alone. Co-Authored-By: Claude <noreply@anthropic.com>
make_icons.py now takes icon names on the command line and fetches them
from the same server the stock firmware pulled from at runtime:
python tools/make_icons.py coffee bread-slice
Names come from https://pictogrammers.com/library/mdi/. Doing this at
build time rather than on the device keeps the runtime free of HTTP, certs
and SPIFFS eviction, while still giving access to the whole icon set. A
name that does not exist fails the build instead of becoming a placeholder
glyph noticed later on the device. A plain run stays offline and
reproducible: plus and minus are still drawn locally.
Also switched the local drawing from 24-bit to 1-bit BMP. Every icon the
server returns is 1-bit, so the 24-bit output was sending locally drawn
icons down a branch of draw_bmp() that upstream's own icons never
exercised - a needless difference in a path not yet tested on hardware.
Output now matches the server byte for byte in structure: 574 bytes at
64px against 12342 before, offset 62, palette [black, white], set bit
meaning white.
Co-Authored-By: Claude <noreply@anthropic.com>
Three changes that belong together. Icons are now listed declaratively in icons.txt next to platformio.ini, one name per line. CI runs make_icons.py with no arguments, so whatever is in that file lands in the published artifacts - previously extra icons could only be passed on the command line, which meant CI never built any. Added food-drumstick and dog-service. SVGs come from the canonical Templarian/MaterialDesign repository, pinned to commit 2424e748e0cc, and are rasterised locally with ImageMagick rather than pulled pre-rendered from the stock firmware's icon CDN. That removes the dependency on a third party's hobby infrastructure staying up and makes the output reproducible from a pinned ref. Verified against the old CDN's rendering of food-drumstick: 2.1% of pixels differ, all of it edge antialiasing either side of the threshold. Costs an ImageMagick dependency at build time; the device is unchanged and still downloads nothing. The counter total was rendering in helvB24 like any other label, which left a two-digit number adrift in a cell nearly 100px tall. An all-digit label now steps down a ladder of large numeric fonts - logisoso42, logisoso32, helvB24, helvB18 - taking the first that fits half the display width, since the two counters share a row. Numbers are never trimmed: "12..." is a wrong value, not a shortened word, so an implausibly long count runs small instead. Costs 1.4 kB of flash for the two extra fonts. Co-Authored-By: Claude <noreply@anthropic.com>
Icons rendered as the unknown-glyph placeholder even with the image flashed. The image was fine; nothing could read it. PlatformIO packs the data directory with mkspiffs_espressif8266_arduino, which is compiled with the ESP8266 Arduino SPIFFS parameters. This project runs the ESP-IDF SPIFFS driver, configured CONFIG_SPIFFS_OBJ_NAME_LEN=56 and CONFIG_SPIFFS_META_LENGTH=4 against that tool's 32 and 0. The object index layouts differ, so the device mounted the partition, enumerated no files, and every mdi: label fell through to file_question_outline. Same 655360-byte image, 3650 bytes different in content. ESP-IDF ships spiffsgen.py for this. Regenerate the image in place as a post-action on $BUILD_DIR/spiffs.bin, so -t buildfs and -t uploadfs keep working normally rather than needing a separate command nobody remembers. Parameters are read out of the generated sdkconfig and the partition size out of partitions.csv, so neither can drift from what the firmware was compiled to read - this fork has already moved that partition once. Upstream never hit this: it downloaded icons at runtime into a filesystem the device had formatted itself, so no image was ever packed on a host. Baking the icons in at build time is what exposed it. Verified: the regenerated image is byte-identical to a manual spiffsgen.py run with the same parameters. Co-Authored-By: Claude <noreply@anthropic.com>
Flashing the debug build left the device hung - enumerating as 303a:0002
and not answering esptool on either a normal or a bootloader boot.
esp_system/Kconfig declares:
config ESP_CONSOLE_USB_CDC
depends on (IDF_TARGET_ESP32S2 || IDF_TARGET_ESP32S3) && !TINY_USB
with the comment "the ROM CDC driver is currently incompatible with
TinyUSB". This project builds framework = arduino, espidf, and Arduino
brings TinyUSB in, so that dependency can never hold here.
Forcing the symbol from a defaults file overrides the unmet dependency
rather than failing the build. It compiled, and the symbol really was
present in the generated sdkconfig - which is exactly what I checked, and
why this got through. On hardware the ROM CDC console and TinyUSB then
both drive the same USB OTG peripheral and the device hangs at boot.
Checking that a config symbol is set is not the same as checking it is
valid; the dependency is the thing that mattered. The file now carries
that reasoning so the option is not reintroduced.
Console is back on UART0 for both builds, read from TX and GND on the
CMSIS-DAP header. The release build was never affected - it kept
CONFIG_ESP_CONSOLE_UART_DEFAULT=y throughout, so it is the safe image to
recover with.
If USB logging is wanted, the supported route is Arduino's own stack, not
the IDF console: -DARDUINO_USB_CDC_ON_BOOT=1 plus
Serial.setDebugOutput(true), leaving one owner of the peripheral. Not
attempted here, and not to be merged without a hardware test.
Co-Authored-By: Claude <noreply@anthropic.com>
Icons and text use different vertical placement in draw_main(). A full size icon sits at a fixed top per row - 17, 116, 215 - shared by both columns, so icons line up across the display. Text is placed at HEIGHT/12 + i*HEIGHT/6, which steps per button index rather than per row and yields 25, 74, 123, 172, 222, 271: the left column high, the right column low, alternating down the screen. That is defensible for captions, where each label just needs to sit near its own button. It is wrong for two counter totals that are meant to read as a pair, and it also made the number jump vertically whenever the digit count pushed it onto a different font, because the baseline was derived from the font ascent. A numeric label is now positioned exactly where a full-size icon would be: one vertical centre per row shared by both columns, centred within the column rather than flush to the edge. Titles and any other text keep the original behaviour. Also collapsed the icon placement itself onto a shared MDI_ROW_TOP_Y constant. Upstream spelled it out as three identical if/else pairs on i with the row tops as magic numbers, which is what made the difference from the text path easy to miss in the first place. Co-Authored-By: Claude <noreply@anthropic.com>
The counter moved in RAM the moment a button was pressed, but the redraw that showed it only ran inside AwakeModeIdleState and SessionState. A press from sleep goes SleepModeHandleInput -> NetConnectingState -> SessionState, and neither of the first two redrew - so the number on the display did not move until Wi-Fi and TLS were up, six to ten seconds later. The screen was reporting delivery while pretending to report the press. Moved the redraw into the main loop as _service_display(), so it follows the press whatever the state machine is doing, and dropped the two per-state copies. Fixed the coalescing while moving it. Both copies updated last_m_display_redraw_ on every pass of the outer interval check, not just when something was drawn, so the timestamp ticked continuously in the background and any press could wait out most of an interval before being shown. The timestamp now moves only on an actual draw, so the first press after an idle spell redraws immediately and only a genuine burst gets coalesced. The e-paper refresh runs in the display task, so it now overlaps the network work rather than queueing behind it. Co-Authored-By: Claude <noreply@anthropic.com>
The counters accumulated forever. They now clear on a configurable
boundary, without the device needing SNTP, a timezone, or an RTC chip.
Time comes from the receiver. Every response carries a UTC epoch and the
current offset in seconds, so any request doubles as a clock sync and DST
stays where the calendar already lives. A bare {"event":"time"} POST
exists for when there is nothing else to send; the receiver's press branch
ignores it, and it is only sent when the clock is older than
CLOCK_RESYNC_SECONDS and no press is queued.
The clock survives deep sleep via the RTC timer, so the boundary is
testable before the network is up. That matters twice: the reset wake can
act immediately, and _handle_counter_press() tests the boundary before
applying, so a press just after 03:00 counts toward the new period rather
than the old one. Getting that ordering wrong would have had the device
show a cleared count while reporting yesterday's total.
Schedule lives in one portal field so four modes need one input:
off | daily 03:00 | weekly mon 03:00 | monthly 1 03:00
Parsed into a canonical form on save, so a typo cannot quietly disable the
reset. Monthly is capped at day 28 - 29-31 would skip short months, which
is a surprising way for a reset not to happen.
reset_schedule reduces all three modes to one integer period per instant,
so the reset test is an equality check rather than date arithmetic at the
call site, and seconds_until_next() feeds the existing but until now
unused flags().schedule_wakeup_time. The reset wake replaces the heartbeat
rather than adding to it: one wake per day that clears, redraws, reports
battery and resyncs the clock in a single round trip.
Guards: a clock older than CLOCK_STALE_SECONDS is not trusted enough to
reset on, and last_reset_period == 0 adopts the current period without
clearing, so setting up a device does not wipe a count it was just given.
SCHEDULE_WAKEUP_MAX caps a sleep at 24h, so weekly and monthly wake daily
and re-evaluate instead of drifting for a month on an RC oscillator.
One event reports the whole reset - {"event":"reset","counts":{...}} -
rather than one request per counter.
Co-Authored-By: Claude <noreply@anthropic.com>
Three fixes from the review of this branch. The blocker: two tasks shared one HTTPClient. _net_on_connect() is invoked from ConnectedState::entry() on the network task, and posted a heartbeat, time sync or reset from there - while _flush_pending() posts presses from the main task. Network sets W_CONNECTED in WifiConnectedState::entry(), one state earlier, so the main task could start a POST inside the window before the callback fired. Two tasks on one WiFiClientSecure is corruption, not a race you get away with. Upstream guarded the equivalent with a task affinity check in Network::publish that went away with MQTT. _net_on_connect() now only raises an atomic flag. All webhook access moved to _service_webhook() on the main task, driven from the main loop next to _service_display(). Counter state was mutated from three tasks - main at boot, UI on press, network on connect - while the main task ran save_all(). The network path is gone with the above; the remaining main/UI overlap is now guarded by a recursive mutex around _handle_counter_press(), _check_reset() and the save. reset_to_report_ became std::atomic, matching what inflight_ and send_failed_ already do. reset_schedule is now free of Arduino, Logger and config.h so it builds on the host, and has 19 unit tests behind `pio test -e native`. It is ~200 lines of calendar arithmetic whose bugs surface weeks later as "the counter cleared on the wrong day", which is exactly what should not need hardware to exercise. parse() reports failure through an out-param instead of logging, and format() writes into a caller buffer. Device settings moved from [env] into [esp32_base] so the native env does not inherit a board and framework it cannot use. Also: sync_time() now takes a sequence number rather than reusing the last one, and _sleep_or_restart() schedules the reset wake instead of falling back to the heartbeat interval. Verified: 19/19 tests pass, both firmware envs build (70.2% / 72.8%). Co-Authored-By: Claude <noreply@anthropic.com>
Seconds rather than minutes, and a calendar bug in reset_schedule would otherwise only surface on a device weeks later. Ordered first so a logic failure does not wait on a full ESP-IDF build. Co-Authored-By: Claude <noreply@anthropic.com>
Firmware and SPIFFS are flashed separately, so they can silently drift apart - and after a run of changes to both there was no way to answer "is this even the build I flashed" without diffing binaries. pre_script.py compiles the short sha in as BUILD_SHA; make_icons.py writes the same value to /build.txt inside the SPIFFS image. Both mark a tree with uncommitted changes as "+dirty", so a hand-built image is never mistaken for a tagged one. The device reports both: at boot in the log, on the Device Info screen as "fw <sha>" and "fs <sha>", and in every webhook payload as `build`. A mismatch between the two is logged as a warning rather than treated as an error - it usually just means a forgotten uploadfs, and the device runs fine either way. __file__ is not reliably defined inside a PlatformIO extra script, which is why the first attempt stamped "nogit"; the project directory now comes from SCons instead. Artifacts are named homebuttons-original-<sha> and homebuttons-original-debug-<sha>, so successive downloads no longer pile up as "firmware-original (3)" with nothing to distinguish them, and build.txt ships in the bundle so an unzipped folder identifies itself. Checkout needs fetch-depth 0 for the sha to be available. Co-Authored-By: Claude <noreply@anthropic.com>
The first run named its artifacts homebuttons-original-cb26e6d2 while the branch head was aad92ec. On a pull_request event actions/checkout builds a synthetic merge commit, so git rev-parse HEAD returned a sha that exists nowhere in the branch - and that sha was being compiled into the firmware and written into the SPIFFS image, making the whole point of the stamp unlookupable. Check out the pull request head instead. CI now tests the branch as-is rather than merged with the base, which is the right trade for something whose job is to say which commit produced this binary. Co-Authored-By: Claude <noreply@anthropic.com>
DeviceState has one Preferences handle, and it is written from two tasks: the network task saves in WifiConnectedState::entry() and NormalConnectState::loop(), while the main task saves after a press and before sleep. Preferences keeps its NVS handle in the object, so interleaved begin()/put()/end() from two tasks corrupts it. The collision is not occasional. WifiConnectedState::entry() is also where state_ becomes W_CONNECTED, and _flush_pending() on the main task saves the moment it observes that - so both tasks enter the handle on essentially every connection. wifi_quick_connect and the saved BSSID live in that namespace, which is why this presents as Wi-Fi that works sometimes: a corrupted quick-connect record sends the device down the 5s quick path to nothing, and a corrupted persisted block can be worse. NVS writes also suspend flash-resident code while they run, which is not something to be doing on the network task at the instant the link comes up. The two-task split is upstream's, but _flush_pending() turned a rare overlap into one per connection, and the mutex added earlier covered main-vs-UI only. Guarded inside DeviceState rather than at the call sites: there are twelve of them across two tasks, and the next one added would have to remember. save_all()/load_all() hold the lock across both halves so no reader observes a half-written pair. Co-Authored-By: Claude <noreply@anthropic.com>
A router that auto-selected channel 13 stopped appearing in the setup portal's scan list at all, which reads as the network having disappeared rather than as a regulatory limit. Nothing in this build set a Wi-Fi country, so the ESP-IDF default applied: "CN" with 802.11d ENABLED. That policy takes the permitted channel range from the country the connected AP advertises, and reverts to the configured one on disconnect - so a device that cannot yet see the AP never gets to learn that the upper channels are allowed. Chicken and egg, and it silently costs you channels 12 and 13. New portal field "Wi-Fi Country" holding an ISO code, applied through esp_wifi_set_country_code(cc, false). Passing false disables 802.11d, so the configured range is used always rather than being inferred and reverted. Applied in all three places that bring the radio up: both connect paths in network.cpp, and - the one that actually matters here - before the setup portal scans. Blank keeps the current behaviour. UA is not among the codes ESP-IDF accepts; PL or another neighbouring EU code gives the same 1-13 range. Documented in the guide along with the symptom, since "my network is not in the list" is not an obvious search term for a regulatory domain setting. Co-Authored-By: Claude <noreply@anthropic.com>
It was only registered in the full setup portal (settings button 1). The Wi-Fi-only portal (button 2) registers no parameters at all, so the field was invisible in exactly the flow where it is needed - you reach that portal because a network is missing from the scan list, and the setting that fixes the scan was on the other screen. That portal has no save-params callback, so the value is read back by hand after the portal closes and saved before the reconnect uses it. Note the scan inside the portal still uses the previously stored country: the value cannot be applied until it has been entered. So on the first visit type the SSID by hand, save, and the scan is correct from the next boot onwards. Co-Authored-By: Claude <noreply@anthropic.com>
Two things about setting the region were unclear on the device. First, changing it now restarts immediately instead of carrying on to the connect. The scan that just ran used the OLD region, so the network list you picked from was incomplete anyway, and changing the country switches the PHY init data - applying that to a live radio and connecting in the same breath is not dependable. Restarting applies it cleanly and the next visit scans correctly. The screen says so rather than just rebooting. Second, the setting was reported as not surviving a reboot and the save path reads correctly, so the device now logs it at each step: what the params page submitted, what putString wrote and how many bytes, and what came back on load. NVS putString fails silently when a namespace is full, and a setting that quietly does not persist is indistinguishable from one that was never entered. Co-Authored-By: Claude <noreply@anthropic.com>
Serial log showed load wifi_cc='' after a save that the browser confirmed, so the value was never reaching NVS. start_wifi_setup() read the field only after its portal loop exited, and that loop exits on a connection attempt, a button press, or the 600s timeout - never on a params save. Entering a region and saving it on its own therefore left the value sitting in the WiFiManagerParameter and read by nobody. Saving the Wi-Fi form did end the loop, which is why the AP disappeared, but WiFiManager had already tried to connect using the old region by then. The portal now has its own save-params callback, so the region is captured the moment the form is submitted. It sets a flag rather than restarting inside the request handler, so the browser still gets its "saved" page, and the loop then restarts to apply it. Also corrected the save log: putString returns strlen(), so the "(0 bytes)" on an empty value looked like a failed write when it was simply an empty string. Co-Authored-By: Claude <noreply@anthropic.com>
A failed association is currently just a 20s timeout, which looks identical whether the AP was never seen, refused us, or rejected the key. The driver knows which; nothing was asking it. Registers handlers for STA_DISCONNECTED and STA_CONNECTED. The disconnect handler logs the numeric reason with a decoded name, so the distinction that matters is in the release build's log rather than needing CORE_DEBUG_LEVEL=5. NO_AP_FOUND, AUTH_FAIL and 4WAY_HANDSHAKE_TIMEOUT point at three completely different problems. The connect handler logs the channel and RSSI actually used, which also confirms whether the region change took effect. Co-Authored-By: Claude <noreply@anthropic.com>
Saving a region took down the setup hotspot mid-session: the callback saved, blocked three seconds on a display update, then rebooted, so the phone sitting in the portal lost the AP and had to find it again. The restart was my guess at what the PHY-init-data note in the esp_wifi_set_country_code docs implied. It is not needed. The region is picked up where it actually matters - apply_wifi_country() already runs before the connect attempt at the end of the portal, and on every connect after that. Applying it live inside the callback is not an improvement either: it rewrites the SoftAP's country IE and can move its channel, dropping the same client for the same reason. So the callback now just saves. One portal session: set the region, pick or type the network, save, and the connect uses the new region. No reboot, no AP disruption. Also logs when the region is unchanged - the previous behaviour returned silently, so submitting the form with an already-correct value looked identical to it being ignored. Co-Authored-By: Claude <noreply@anthropic.com>
Flashing currently needs the BOOT+RST dance because the running firmware puts nothing on USB, so esptool has no line to pull. Arduino's USBCDC already implements the DTR/RTS state machine esptool drives - including usb_persist_restart(RESTART_BOOTLOADER) and the 1200-baud touch - it just has to be enabled. esptool --chip esp32s2 --after hard-reset chip-id pio run -t upload This is NOT the setting that bricked the board earlier. That was CONFIG_ESP_CONSOLE_USB_CDC, the ESP-IDF ROM CDC driver, whose Kconfig reads "depends on !TINY_USB" - it and TinyUSB were fighting over one peripheral. This is Arduino's own TinyUSB CDC, the single owner and the documented path, enabled by a build flag the core explicitly supports: main.cpp calls Serial.begin()/USB.begin() for it, and ARDUINO_USB_MODE is undefined on S2 so the guard passes. The ESP-IDF console stays on UART0 - verified in the generated sdkconfig - so the debug header keeps working unchanged and USB is purely for reset and upload. Costs 0.6% of flash. Deep sleep still tears the USB device down, so this only helps while the device is awake. Untested on hardware. Co-Authored-By: Claude <noreply@anthropic.com>
The interesting behaviour of this firmware is otherwise only reachable by standing at the device with a finger on a button, or by waiting until 03:00 tomorrow. `press` injects through the real handler and `time set` moves the clock, which together make the counter flow and the scheduled reset testable over the wire in seconds. Reading and executing are split. The main loop blocks for up to HTTP_TIMEOUT inside a POST, so a reader task buffers input meanwhile and typed characters are not lost to a UART FIFO nobody is draining. Execution then happens on the main loop, which already owns the webhook, the NVS writes and the counters - running commands anywhere else would put a second task on all three. Both ports are read: Serial is the native USB CDC and Serial0 is UART0 on the debug header. Which one is attached depends on how the device is powered, and neither is worth privileging. An injected press also nudges the state machine the way the UI callback would have. Without that, a press in sleep mode would sit in the queue until the idle timeout slept the device with it undelivered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFniCJ94a3sciMP9MtbPfs
_check_reset() fired on any change of period, which meant a backwards step counted as a boundary crossing. Found on hardware: switching the device to a receiver reporting a different tz_offset moved local time across 03:00 and wiped the counters. The case that matters in service is the autumn DST step. Handing back an hour at 03:00 local, against a "daily 03:00" schedule, would clear and notify on the way down and again on the way back up. A period being re-entered has already had its reset, so adopt it without clearing. Forward crossings are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFniCJ94a3sciMP9MtbPfs
_check_reset() was reachable from a press, from the on-connect handler and from boot. In sleep mode that is enough - the scheduled wake is the trigger and the check at boot covers it. A device left awake has none of those, so 03:00 passed with the counters untouched until somebody pressed a button. That is not an edge case: anything on USB power with awake mode on runs this way, which is how the device sits on a desk. Verified on hardware before and after. With a boundary set two minutes out and no input, the counters stayed at 5 and 3 and last_reset_period never advanced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFniCJ94a3sciMP9MtbPfs
Toggling awake mode was portal-only, which made the deep sleep path - the one a battery device actually runs - awkward to exercise. Needed to validate that a scheduled reset wake fires. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFniCJ94a3sciMP9MtbPfs
CmdShutdownState::entry() flushed queued presses and then commanded a disconnect, but the scheduled-reset report and the heartbeat go out from _service_webhook(), which runs after loop(). By the time it started its POST the link was already going down. Observed on a timer wake: reset body at t+309ms, "disconnecting..." at t+319ms, then three DNS failures and "post failed after 3 attempts" at t+22540ms. The notification was lost and the device stayed awake 22s to lose it - on battery, that is the expensive part. Service the webhook from the shutdown entry instead, covering all three kinds of pending work, and latch a flag afterwards. The network state only flips to DISCONNECTED when the network task gets round to it, so W_CONNECTED lingers a few ms past the disconnect command - long enough to start a POST that cannot succeed. Nothing is lost by giving up at that point: every event carries absolute counts, so the next one repairs the receiver. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFniCJ94a3sciMP9MtbPfs
… one Two faults, both only reachable after a reset that is not a deep sleep wake. System time goes back to 1970 while last_time_sync keeps a real epoch in NVS. The resync test subtracted one from the other and asked whether the result exceeded the resync interval. With a 1970 clock that difference is around minus fifty-six years, which is not greater than six hours, so the device concluded its clock was current and never asked for the time again. The daily reset stays suspended from then on, because _clock_fresh() - correctly - refuses to trust it. _schedule_next_wake() then made it worse by guarding on clock_valid() rather than the same freshness test, so it scheduled from the 1970 clock anyway. Observed: a boundary 150 s away turned into a 7.3 hour sleep. Falling through to no scheduled wake uses the heartbeat interval, and the heartbeat is what re-syncs the clock. Also adds `sleep <secs>` to the console. Testing the sleep path meant guessing how long the device would be gone; an explicit wake keeps it reachable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFniCJ94a3sciMP9MtbPfs
An unreachable receiver costs HTTP_MAX_ATTEMPTS * HTTP_TIMEOUT plus backoff per send, around 31 s. A heartbeat followed by a reset report is two of those against a 60 s watchdog, with nothing feeding it in between. _flush_pending() already resets per press; these two did not. Also corrects a comment: changing the endpoint needs no client teardown, because _post() reads the URL from state on every attempt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFniCJ94a3sciMP9MtbPfs
The response contract was the least obvious thing about this firmware and the easiest to get wrong: the device has no RTC and no NTP, so a receiver that answers 200 with a plain-text body leaves it counting correctly and permanently unaware of the time. Both ways that happens in n8n - an unpublished draft, and $now.offset reporting the instance timezone rather than a named zone - are now written down, with a curl that checks it from outside the device. Also documents the console command set and the esptool first-attempt failure, which is expected rather than a fault. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFniCJ94a3sciMP9MtbPfs
…okkeeping The shutdown fix in ce72fab traded one bug for two worse ones. entry() runs on whichever task made the transition, and several handle_ui_event handlers reach CmdShutdownState from the UI task - so the blocking POST it added could attempt a TLS handshake on the UI task's smaller stack, and put a second task on the one HTTPClient the main task owns. Latching shutting_down_ there made it worse: the network task raises the connect event a moment after the link comes up, so a timer wake reaching shutdown first skipped its heartbeat and then had every later attempt suppressed by the latch. No heartbeat means no clock refresh, and after 48 h the daily reset stops running with nothing to show for it. Draining now happens in CmdShutdownState::loop(), which only ever runs on the main task, and holds the link until _webhook_pending() clears or the drain timeout expires. Reset bookkeeping, both from the same root cause: - Adopting the earlier period on a backwards clock re-armed a boundary that had already fired, so crossing it again cleared a second time and posted a second report, wiping every press in between. Hold the high-water mark instead. - That is only safe if a change of schedule zeroes the stored period, since period_of() counts days for daily but weeks or months otherwise. set_reset_spec() now does it, which also fixes the setup portal silently skipping the first reset after a schedule change - the console did it by hand and the portal did not. Also: _service_reset() persists after a crossing, so an overnight reset on an always-awake device is not lost on the next boot; the console feeds the watchdog between commands, since two blocking sends pasted together outlast it; the three console commands that wrote DeviceState without StateLock now take it; a console press hands the state transition to SleepModeHandleInput::loop() rather than driving the unsynchronised state machine from a second task; and `sleep <secs>` arms the timer it reports even in the states the normal path excludes, which are the ones where being stranded costs the most. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFniCJ94a3sciMP9MtbPfs
Two of the review findings were mistakes in three lines of comparison logic, and neither was reachable from a test: the rule lived inside App::_check_reset(), behind a FreeRTOS task, a mutex and a clock. The module it belongs to already had 19 host tests and none of them could touch it. reset_schedule::decide() now takes the stored and current period and returns what to do. _check_reset() only carries it out. Seven cases added, each either a defect that shipped or the invariant it broke. Confirmed they bite: reintroducing the adopt-on-backwards bug fails three of them, including the DST walk asserting one clear per boundary - "Expected 1 Was 2" is the double-clear exactly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFniCJ94a3sciMP9MtbPfs
It shipped in release with no gate and no authentication. Anyone with physical USB-C or debug-header access could point the device at another receiver with `endpoint` and `token`, or reopen the provisioning portal with `setup`. platformio.ini already defines HOME_BUTTONS_DEBUG for env:original_debug and setup.cpp uses it for exactly this purpose. Guarding the whole translation unit rather than the call sites keeps the command table and its handlers out of the image entirely: no Console symbols and no command strings survive in original_release, confirmed with nm and strings. Release drops 26 KB of flash and 232 bytes of static RAM, plus the 3 KB reader stack and the queues that begin() would have allocated. The trade is real - the console is what made the counter and reset flows testable at all - so it is one build flag away, and the guide says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFniCJ94a3sciMP9MtbPfs
Both predated the scheduled reset and the console, and a few claims had drifted from what the code does. Corrections that would have misled someone following along: - The guide's verification steps opened with "press button 1", which is a title button and does nothing. Now button 3, with the decrement and the do-nothing case as their own steps. - The LED was described as blinking on a press. It lights and stays lit until the receiver answers, which is the whole point of it: solid means pending, dark means delivered, three blinks means it failed. - Session timing said ~200 ms from the original estimate. Measured on hardware it is about 130 ms, with the first POST at 4.2 s. - Flash budget said 69.3%, now 71.0%. Undocumented until now: the Counter Reset portal field and the schedule behind it, the reset event payload, the `build` key, and the fact that the webhook response is the device's only clock. That last one is the expensive gap, because nothing looks wrong when it is missing. esptool commands use the v5 hyphenated spelling, in the guide and in the two tools under tools/, with a note for anyone still on v4. Also drops a line in the compile-time table that referred to a private conversation rather than saying what the setting does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFniCJ94a3sciMP9MtbPfs
The guide used /dev/ttyACM0 as the port and said nothing about picking it. With a USB-to-serial adapter also attached, the numbering is whatever order the kernel enumerated things, and the adapter is an inviting wrong answer: it reaches the ROM bootloader over UART0, so esptool connects and reports the chip correctly before failing later for reasons that look unrelated. What actually happens on an adapter is that the baud change succeeds and the next read times out with "Unable to verify flash chip connection", and that --before default-reset cannot work at all because DTR and RTS are not wired to EN and IO0. Both are now written down, with a udevadm one-liner that names the ports. Also drops --baud from the flash command. On the Espressif port it is a native USB CDC and the rate is ignored, and asking for 921600 is exactly what breaks the adapter. Notes that all five images are needed. Omitting 0x10000 firmware.bin flashes cleanly and boots, and leaves the previous application running. The reflash section no longer repeats the retry explanation; it points at section 1 so the two cannot drift. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFniCJ94a3sciMP9MtbPfs
The guide said the first attempt fails and to retry, which is true of one failure and misleading for the others. Retrying a wedged chip four times achieves nothing, and the fix for each is different. Write timeout means a stub flasher is still loaded from an earlier command that died partway, and only a chip reset clears it. A failure immediately after "Changing baud rate" means the link cannot carry the rate, which is what a serial adapter does. Also records that --before no-reset is only safe when the chip is known to be freshly in download mode. The USB product string does not distinguish a clean ROM bootloader from a chip running an abandoned stub, so default-reset is the better default. And that a chip reset means the cable or BOOT+RST. Resetting the USB device from the host does not reset the chip, and deauthorising the port can drop it off the bus with no way back except the cable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFniCJ94a3sciMP9MtbPfs
impuls42
added a commit
that referenced
this pull request
Jul 31, 2026
…ry to the Wi-Fi portal (#2) Three fixes from using the device rather than reading the code, after #1 landed. ## Wi-Fi Country is now on the Wi-Fi page too It was only on the Setup page, filed alongside the webhook URL and the six button labels. It is the setting that decides which channels a scan returns, so reaching it meant leaving the list of networks that was missing the one you were looking for. `setParamsPage()` bundles two unrelated things: it sets `_paramsInWifi`, which gates whether parameters render *and* save on the Wi-Fi page, and it swaps the menu. Passing `true` confined every parameter to Setup. There is no per-parameter placement, so one field cannot be moved on its own. What the flag does not gate is the Setup page, because `handleParam()` renders unconditionally. Turning the flag on and putting `param` back in the menu by hand gives both pages, and a save from either commits everything. Wi-Fi Country is registered first so that on the Wi-Fi page it lands under the network fields rather than below the button labels. One behaviour change: saving from the Wi-Fi page now runs `save_params_callback`, which sets `web_portal_saved_` and ends the portal. It did not before. That is the right outcome after changing networks, and since parameters are seeded from current state, a save that touched nothing writes back what was already there. ## The status screen shows the clock and the build Hold any single button for 2 s from idle: ``` - Battery - [icon] 87 % 3.92 V DC power 2026-07-27 16:25 build aa99f38a ``` The time is local, using the offset the receiver last reported. `clock not set` in its place is the visible symptom of a receiver that is not returning `ts` and `tz_offset`, which otherwise shows up only over serial and quietly stops the scheduled reset. The build line is one line while the firmware and SPIFFS stamps agree and splits into `fw` and `fs` when they do not, so a forgotten `uploadfs` is obvious. Both were already on the device info screen, but that is three steps into the settings menu and is not where anyone looks. ## Verification Both environments build, 26 host tests pass. The screen change is draw-only and the portal change is parameter registration, so neither touches the counter, webhook or reset paths. Not yet exercised on hardware. The portal change in particular is worth a look in a browser before merging. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Turns the Home Buttons Original (A1) into a two-channel tally counter. Each press POSTs to an HTTPS webhook instead of publishing over MQTT.
Base is this fork's
master, not upstreamnplan/HomeButtons. Nothing here is meant to go back upstream.Behaviour
The buttons are two columns of three, one counter per column. Row 1 is a label or icon, row 2 increments and shows the running total, row 3 decrements.
A press lights the LED, increments the counter, redraws the display and queues a POST. The LED clears once the request is delivered, so it reports delivery rather than just that the device is awake. The display never waits on the network.
The device is authoritative for the counter. Every request carries the absolute value alongside the delta, so a dropped request repairs itself on the next press and there is no retry queue to fall out of sync.
seqexists so the receiver can dedupe a retry and avoid notifying twice.After the first press the Wi-Fi association and TLS session stay open for
SESSION_IDLE_TIMEOUT, so a burst of presses shares one handshake. Measured on hardware, the first POST takes 4.2 s and later ones about 130 ms.Button callbacks run on the UI task, so
handle_ui_event()only touches RAM and pushes onto a FreeRTOS queue. The main task owns the NVS write and the POST, and is the only task that touches the HTTP client.Scheduled counter reset
One free-text field sets when the counters clear:
off,daily 03:00,weekly mon 03:00ormonthly 1 03:00. The device schedules its deep sleep wake for the next boundary, clears on the way back up, and reports the cleared totals.A period is cleared at most once. Local time moving backwards over a boundary (the autumn DST step, or a clock correction) holds the stored period rather than re-arming it, and changing the schedule zeroes it, because
period_of()counts days for daily but months for monthly.reset_schedule::decide()is a pure function so these cases have unit tests.If the clock is older than
CLOCK_STALE_SECONDSthe reset suspends instead of guessing.The webhook response carries the clock
The device has no RTC and no NTP. Its only source of time is the webhook reply, which has to be JSON with an absolute UTC epoch and the local offset in seconds:
{"ts": 1785114718, "tz_offset": 7200}Without it the clock never becomes valid and the scheduled reset never fires, while presses and notifications carry on working normally. Two ways that goes wrong in n8n, neither of which shows up as an error: serving an unpublished draft, which sends the device
Workflow got started.as plain text, and leaving$now.offsetto report the instance timezone, which is UTC on a default install.docs/counter-guide.mdcovers both, with a curl that checks the contract from outside the device.Serial console
Debug builds carry a line-oriented console on UART0 and USB CDC.
press <n>injects through the real press handler,time set <epoch>moves the clock,sleep <secs>forces a wake time, andstatusdumps build stamps, counters, clock and schedule. Without it the counter and reset flows can only be tested by standing at the device or waiting until tomorrow.Release images do not include it. It can rewrite the endpoint and auth token with no authentication beyond physical access, and it costs 26 kB of flash and about 4 kB of RAM.
Removed
tools/make_icons.py, pinned to a Templarian/MaterialDesign commit, and ship in the SPIFFS image.pro_*targets could not compile and had not been able to for some time:Apphas nobsl_input_member for that variant,touch.cppemits enum values that do not exist, andset_frontlightis declareduint8_tbut defineduint16_t.TLS
Uses the ESP-IDF certificate bundle (CMN, 14.7 kB) rather than a pinned root. The pinned root was the DST cross-signed ISRG Root X1, which expired 2024-09-30 while the comment claimed it was valid until 2030. A custom icon server was separately passed
nullptras the CA, so HTTPS with no verification at all. The backend sits behind Cloudflare, which rotates edge issuers, so a bundle is the only durable option.Flash
SPIFFS shrinks from
0x160000to0xA0000and the app slots grow from0x140000to0x1A0000. The baseline build used 95.8% of its app partition. This one uses 71.0% including the cert bundle. CI fails the build above 90%.Other fixes, all present upstream at v2.6.1
__builtin_ctzllfor wake-pin decodelog(mask)/log(2)on floats, wrong pin with two buttons held and undefined for a zero maskmillis()against the timeoutsizeof()in_nvs_2_efuse()char[9], a struct overflow_efuse_burned()bufferuint8_t buf[8], read 8 bitsHardwareDefinitiongetStringdoes not guarantee NUL termination at the limitload_persisted()"password123"on every unit shippedCI
Upstream had no firmware build, only a docs deploy, which is how the
pro_*targets drifted into not compiling unnoticed. This adds a build gate with a PlatformIO cache, the host unit tests, and a flash headroom check. Artifacts are named by short SHA so a downloaded build can be matched against what is on the device.The
runs-onexpression is ADR-0007 Form 2. This repo is public, so it cannot reach the org ARC runners (allows_public_repositories: falseon the Default runner group) and cannot readCI_RUNNER_MODE, which is a private-visibility org var, so it resolves toubuntu-latest. It picks up ARC automatically if the repo is ever made private.Verification
Flashed and exercised on a real A1:
26 host unit tests cover the schedule arithmetic and the reset decision.
Running it on hardware found six defects that neither the build nor the unit tests could: the reset never firing while the device stayed awake, the clock never re-syncing after a reset (which disabled the scheduled reset permanently and silently), a wake scheduled from an uninitialised clock, the reset POST racing the disconnect and losing the notification, counters clearing more than once across a DST fallback, and the watchdog margin during a webhook retry.