From 49c313f126ea9540107e1d030692272e93cb48f8 Mon Sep 17 00:00:00 2001 From: David Bishop Date: Tue, 2 Jun 2026 09:49:46 -0700 Subject: [PATCH 1/3] Add ADC resistor-ladder binary inputs (analog buttons) BinaryInputs input_type == 2: several buttons share one ADC pin, distinguished by voltage. Polled (no edge interrupt) and reported through the same MSD button byte as digital buttons, so hosts decode one uniform contract. Wire layout (num_buttons, id_base, N+1 LE uint16 descending thresholds in reserved[]) is documented in structs.h. device_control.cpp adds classify/register/poll with sample debounce, per-pin ADC attenuation, a static_assert guarding reserved[] capacity, and rejection of non-descending thresholds from any host. ESP32 only; NRF gets an empty poll stub. Verified on an ESP32-C3 (XTEINK X4): valid ladders register and report presses; a malformed (non-descending) config is skipped at init with a diagnostic while a valid ladder beside it still works. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/device_control.cpp | 132 ++++++++++++++++++++++++++++++++++++++++- src/structs.h | 17 +++++- 2 files changed, 147 insertions(+), 2 deletions(-) diff --git a/src/device_control.cpp b/src/device_control.cpp index f350aaf..5efe318 100644 --- a/src/device_control.cpp +++ b/src/device_control.cpp @@ -47,6 +47,126 @@ struct ButtonState { #define MAX_BUTTONS 32 extern ButtonState buttonStates[MAX_BUTTONS]; +// BinaryInputs.input_type wire values (see structs.h for the full contract). +#define BINARY_INPUT_TYPE_DIGITAL 1 +#define BINARY_INPUT_TYPE_ADC_LADDER 2 + +// --- ADC resistor-ladder buttons (e.g. XTEINK X4) ------------------------- +// Several buttons share one ADC pin via a resistor ladder, distinguished by +// voltage. They have no edge interrupt, so they are polled. Reported through +// the same MSD button byte as digital buttons for a uniform host contract. +#ifdef TARGET_ESP32 +#define MAX_ADC_LADDERS 4 +#define MAX_LADDER_BUTTONS 5 // reserved[] holds at most N+1 = 6 LE uint16 thresholds +#define ADC_LADDER_POLL_MS 5 +#define ADC_LADDER_DEBOUNCE 3 // consecutive equal samples required to accept a change + +// Thresholds for N+1 buttons must fit in BinaryInputs.reserved[]; fail the build if not. +static_assert(2 + 2 * (MAX_LADDER_BUTTONS + 1) <= sizeof(BinaryInputs::reserved), + "ADC ladder thresholds would overflow BinaryInputs.reserved[]"); + +struct AdcLadder { + uint8_t pin; + uint8_t num_buttons; + uint8_t id_base; + uint8_t byte_index; + uint16_t thresholds[MAX_LADDER_BUTTONS + 1]; // descending; [0] = idle ceiling + int8_t current_button; // -1 = none pressed + int8_t candidate_button; // debounce: last raw classification + uint8_t candidate_count; // consecutive samples equal to candidate + uint8_t press_count; // 0-15, increments per press (5 s reset window) + uint8_t last_button_id; // id of most recent press (for clean release reporting) + uint32_t last_press_time; +}; +static AdcLadder adcLadders[MAX_ADC_LADDERS]; +static uint8_t adcLadderCount = 0; + +// Returns button index 0..num_buttons-1, or -1 when nothing is pressed. +static int classifyAdcLadder(int adc, const AdcLadder* l) { + if (adc > (int)l->thresholds[0]) return -1; // above idle ceiling + for (uint8_t i = 0; i < l->num_buttons; i++) { + if (adc > (int)l->thresholds[i + 1]) return (int)i; // thr[i+1] < adc <= thr[i] + } + return (int)l->num_buttons - 1; // catch-all bottom bucket +} + +static void registerAdcLadder(const struct BinaryInputs* input) { + if (adcLadderCount >= MAX_ADC_LADDERS) return; + uint8_t n = input->reserved[0]; // num_buttons + if (n == 0 || n > MAX_LADDER_BUTTONS) return; + if (input->button_data_byte_index > 10) return; + AdcLadder* l = &adcLadders[adcLadderCount]; + l->pin = input->reserved_pin_1; // ADC GPIO + l->num_buttons = n; + l->id_base = input->reserved[1]; + l->byte_index = input->button_data_byte_index; + for (uint8_t k = 0; k <= n; k++) { + l->thresholds[k] = (uint16_t)input->reserved[2 + 2 * k] | + ((uint16_t)input->reserved[3 + 2 * k] << 8); + } + for (uint8_t k = 0; k < n; k++) { // contract: thresholds strictly descending + if (l->thresholds[k] <= l->thresholds[k + 1]) { // reject malformed config from any host + writeSerial("ADC ladder: thresholds not strictly descending on pin " + + String(input->reserved_pin_1) + ", skipping", true); + return; + } + } + l->current_button = -1; + l->candidate_button = -1; + l->candidate_count = 0; + l->press_count = 0; + l->last_button_id = (uint8_t)(l->id_base & 0x07); + l->last_press_time = 0; + pinMode(l->pin, INPUT); + analogSetPinAttenuation(l->pin, ADC_11db); + adcLadderCount++; + writeSerial("ADC ladder: pin " + String(l->pin) + " n " + String(n) + + " idBase " + String(l->id_base) + " byteIdx " + String(l->byte_index), true); +} + +static void pollAdcButtons() { + if (adcLadderCount == 0) return; + static uint32_t lastPoll = 0; + uint32_t now = millis(); + if (now - lastPoll < ADC_LADDER_POLL_MS) return; + lastPoll = now; + for (uint8_t i = 0; i < adcLadderCount; i++) { + AdcLadder* l = &adcLadders[i]; + int adc = analogRead(l->pin); + int btn = classifyAdcLadder(adc, l); + if (btn == l->candidate_button) { + if (l->candidate_count < 255) l->candidate_count++; + } else { + l->candidate_button = (int8_t)btn; + l->candidate_count = 1; + } + if (l->candidate_count < ADC_LADDER_DEBOUNCE) continue; // not yet stable + if (btn == l->current_button) continue; // no change + uint8_t state; + if (btn >= 0) { + if (l->last_press_time == 0 || now - l->last_press_time > 5000) l->press_count = 0; + l->press_count = (uint8_t)((l->press_count + 1) & 0x0F); + l->last_press_time = now; + l->last_button_id = (uint8_t)((l->id_base + btn) & 0x07); + state = 1; + } else { + state = 0; // released; last_button_id identifies which button + } + l->current_button = (int8_t)btn; + uint8_t data = (uint8_t)((l->last_button_id & 0x07) | + ((l->press_count & 0x0F) << 3) | + ((state & 0x01) << 7)); + if (l->byte_index < 11) dynamicreturndata[l->byte_index] = data; + updatemsdata(); + writeSerial("ADC btn pin " + String(l->pin) + " adc=" + String(adc) + " idx=" + String(btn) + + " id=" + String(l->last_button_id) + " cnt=" + String(l->press_count) + + " state=" + String(state), true); + } +} +#else +static void pollAdcButtons() {} +#endif + void connect_callback(uint16_t conn_handle) { (void)conn_handle; writeSerial("=== BLE CLIENT CONNECTED ===", true); @@ -373,6 +493,7 @@ void handleLedStop(uint8_t* data, uint16_t len) { void processButtonEvents() { powerButtonPoll(); + pollAdcButtons(); if (buttonEventPending) { buttonEventPending = false; uint32_t currentTime = millis(); @@ -531,10 +652,19 @@ void initButtons() { buttonStates[i].pin = 0xFF; buttonStates[i].instance_index = 0xFF; } +#ifdef TARGET_ESP32 + adcLadderCount = 0; +#endif if (globalConfig.binary_input_count == 0) return; for (uint8_t instanceIdx = 0; instanceIdx < globalConfig.binary_input_count; instanceIdx++) { struct BinaryInputs* input = &globalConfig.binary_inputs[instanceIdx]; - if (input->input_type != 1) continue; +#ifdef TARGET_ESP32 + if (input->input_type == BINARY_INPUT_TYPE_ADC_LADDER) { + registerAdcLadder(input); + continue; + } +#endif + if (input->input_type != BINARY_INPUT_TYPE_DIGITAL) continue; if (input->button_data_byte_index > 10) continue; uint8_t* instancePins[8] = { &input->reserved_pin_1,&input->reserved_pin_2,&input->reserved_pin_3,&input->reserved_pin_4, diff --git a/src/structs.h b/src/structs.h index 62b64ef..432beda 100644 --- a/src/structs.h +++ b/src/structs.h @@ -162,9 +162,24 @@ struct DataBus { } __attribute__((packed)); // 0x25: binary_inputs (repeatable, max 4 instances) +// +// input_type == 1 (digital): each set reserved_pin_N is one button on its own +// GPIO, read with digitalRead + edge interrupt. invert/pullups/pulldowns apply +// per pin. All pins in the instance report into button_data_byte_index. +// +// input_type == 2 (ADC resistor ladder, e.g. XTEINK X4): several buttons share +// one ADC pin, distinguished by voltage; polled (no interrupt). Layout: +// reserved_pin_1 = ADC GPIO pin +// reserved[0] = num_buttons N (1..5) +// reserved[1] = button id base (button i reports id base+i, &7) +// reserved[2 + 2k .. +1] = threshold[k], LE uint16, k = 0..N (N+1 values) +// Thresholds descend; button i is pressed when thr[i+1] < adc <= thr[i], and +// nothing is pressed when adc > thr[0] (idle ceiling). thr[N] is the bottom +// floor (use 0). Reported into button_data_byte_index using the same byte +// format as digital buttons. struct BinaryInputs { uint8_t instance_number; // Unique index for multiple input blocks (0-based) - uint8_t input_type; // Input type enum + uint8_t input_type; // 1 = digital buttons, 2 = ADC resistor ladder uint8_t display_as; // How input should be represented in systems (enum) uint8_t reserved_pin_1; // Reserved / spare pin 1 uint8_t reserved_pin_2; // Reserved / spare pin 2 From a012c1dd495249cff80ed518fb042fdb14cb0d87 Mon Sep 17 00:00:00 2001 From: David Bishop Date: Tue, 2 Jun 2026 11:11:42 -0700 Subject: [PATCH 2/3] Reject out-of-range ADC-ladder id_base instead of masking it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit registerAdcLadder stored id_base raw and the poll path masked the reported id with & 0x07. A host sending id_base + count > 8 (3-bit id field) produced wrapped, colliding button ids that HA cannot disambiguate — silently, with no feedback. The firmware is the validation boundary for any host, not just py-opendisplay, so reject it. Also give the existing count and byte_index guards the same diagnostic the descending-threshold check already emits; previously they dropped a malformed config silently. Hardware-verified on XTEINK X4: a GPIO2 ladder with id_base=6, N=4 is now rejected ("exceeds 3-bit id space ... skipping") while a valid GPIO1 ladder registers. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/device_control.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/device_control.cpp b/src/device_control.cpp index 5efe318..cbd600b 100644 --- a/src/device_control.cpp +++ b/src/device_control.cpp @@ -93,8 +93,21 @@ static int classifyAdcLadder(int adc, const AdcLadder* l) { static void registerAdcLadder(const struct BinaryInputs* input) { if (adcLadderCount >= MAX_ADC_LADDERS) return; uint8_t n = input->reserved[0]; // num_buttons - if (n == 0 || n > MAX_LADDER_BUTTONS) return; - if (input->button_data_byte_index > 10) return; + if (n == 0 || n > MAX_LADDER_BUTTONS) { + writeSerial("ADC ladder: count " + String(n) + " out of range 1.." + + String(MAX_LADDER_BUTTONS) + " on pin " + String(input->reserved_pin_1) + ", skipping", true); + return; + } + if (input->button_data_byte_index > 10) { // index into the 11-byte MSD block + writeSerial("ADC ladder: byte_index " + String(input->button_data_byte_index) + + " out of range 0..10 on pin " + String(input->reserved_pin_1) + ", skipping", true); + return; + } + if ((int)input->reserved[1] + n > 8) { // 3-bit id field: id_base..id_base+n-1 must be <= 7 + writeSerial("ADC ladder: id_base " + String(input->reserved[1]) + " + count " + String(n) + + " exceeds 3-bit id space on pin " + String(input->reserved_pin_1) + ", skipping", true); + return; + } AdcLadder* l = &adcLadders[adcLadderCount]; l->pin = input->reserved_pin_1; // ADC GPIO l->num_buttons = n; From 50da6671992b4b8cba1f270a4a38b2fa7478e2e0 Mon Sep 17 00:00:00 2001 From: David Bishop Date: Thu, 4 Jun 2026 11:54:24 -0700 Subject: [PATCH 3/3] Move ADC ladder to input_type 3 (2 is reserved for switches) Per maintainer review on #35: BinaryInputs input_type == 2 is reserved for the host-side switch feature. Renumber the ADC resistor-ladder type to 3 and document 2 = switch in the struct contract. Wire layout and the reused reserved[] fields are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/device_control.cpp | 3 ++- src/structs.h | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/device_control.cpp b/src/device_control.cpp index cbd600b..693d6ce 100644 --- a/src/device_control.cpp +++ b/src/device_control.cpp @@ -48,8 +48,9 @@ struct ButtonState { extern ButtonState buttonStates[MAX_BUTTONS]; // BinaryInputs.input_type wire values (see structs.h for the full contract). +// 2 is reserved for switches (host-side feature); the ADC ladder uses 3. #define BINARY_INPUT_TYPE_DIGITAL 1 -#define BINARY_INPUT_TYPE_ADC_LADDER 2 +#define BINARY_INPUT_TYPE_ADC_LADDER 3 // --- ADC resistor-ladder buttons (e.g. XTEINK X4) ------------------------- // Several buttons share one ADC pin via a resistor ladder, distinguished by diff --git a/src/structs.h b/src/structs.h index 432beda..2e390b9 100644 --- a/src/structs.h +++ b/src/structs.h @@ -167,7 +167,9 @@ struct DataBus { // GPIO, read with digitalRead + edge interrupt. invert/pullups/pulldowns apply // per pin. All pins in the instance report into button_data_byte_index. // -// input_type == 2 (ADC resistor ladder, e.g. XTEINK X4): several buttons share +// input_type == 2 (switch): reserved for the host-side switch feature. +// +// input_type == 3 (ADC resistor ladder, e.g. XTEINK X4): several buttons share // one ADC pin, distinguished by voltage; polled (no interrupt). Layout: // reserved_pin_1 = ADC GPIO pin // reserved[0] = num_buttons N (1..5) @@ -179,7 +181,7 @@ struct DataBus { // format as digital buttons. struct BinaryInputs { uint8_t instance_number; // Unique index for multiple input blocks (0-based) - uint8_t input_type; // 1 = digital buttons, 2 = ADC resistor ladder + uint8_t input_type; // 1 = digital buttons, 2 = switch, 3 = ADC resistor ladder uint8_t display_as; // How input should be represented in systems (enum) uint8_t reserved_pin_1; // Reserved / spare pin 1 uint8_t reserved_pin_2; // Reserved / spare pin 2