ESPSomfy-RTS/docs/SOMFY_SPLIT_PLAN.md
cjkas c0cdf93747 docs: point to the files Somfy.cpp was split into
Add the Somfy.cpp split plan with phases 0 and 1 marked done, what the
split taught about flash size in the debug build, and the phase 2 work
still open. KNOWN_ISSUES.md now names the new files.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-13 20:48:24 +02:00

9.4 KiB
Raw Blame History

Plan: split and refactor Somfy.cpp

Status: phases 0 and 1 done (2026-10-13, commits bbfb23e..505108c); phase 2 open. Written: 2026-10-13, against commit 4544eb6 on fix/live-bugs. Line numbers in the analysis below refer to that commit. The code now lives in the files listed under Phase 1.

Why

src/Somfy.cpp is 4,766 lines. About 690 of them never compile. Six classes, two config structs and the receive interrupt handler share the file. The methods of each class are scattered: SomfyShade's 71 methods are spread over about 20 separate runs. Some logic exists in several copies that have already drifted apart (see Problems).

What the file contains

What Lines Live lines (approx.) Changes when…
Dead code: /* */ blocks and #ifdef USE_NVS blocks (USE_NVS is never defined) everywhere ~690 (392 + 296) never
Radio: Transceiver, transceiver_config_t, receive interrupt and pulse decoder, radio task, frequency scan, RadioLock/TxQueueLock 1–116, 3700–4766 ~950 the radio hardware or RX/TX changes
MQTT: publish*/unpublish* for shades, groups and rooms, plus Home Assistant discovery 980–1348, 3113 ~400 the MQTT topics change
Socket events: emitState/emitCommand 1451–1611, 3105 ~90 the socket protocol changes
JSON config: validateJSON/fromJSON 2775–3066 ~200 the web config form changes
GPIO and dry contacts: setGPIOs, triggerGPIOs, usesPin 570–695, 2758 ~140 the relay wiring changes
SomfyRemote: rolling codes, sendCommandAfter, repeatFrame scattered ~200 how frames are sent changes
Shade behaviour: processFrame, processInternalCommand, processWaitingFrame, checkMovement, my-position, moveTo*, sendCommand 696–923, 1612–2706 ~1,350 what a shade does
SomfyShadeController and SomfyRoom: add/delete/find, ids, commit 117–300, 3076–3700 ~600 the shade list changes

Problems beyond file size

  1. The same logic exists in several copies, and the copies have drifted apart.
    • Step targets. processFrame uses the tested stepPosition() from ShadeMotion. processInternalCommand (~2172–2242) has its own inline maths, which ignores the frame's step count. processWaitingFrame uses a fixed 1% step.
    • Sun/wind. Sensor state is set in the Sensor case of processFrame, acted on by timers in checkMovement, and checked again in the Down case of processInternalCommand (windLast). The SunFlag handling exists twice. These look like real bugs for tilt-only shades:
      • the copy in processFrame (~1865) moves the tilt target of a tilt-only shade;
      • the copy in processInternalCommand (~2262) moves the lift target instead;
      • the sun timer in checkMovement (~744) also moves the lift target of a tilt-only shade, while the no-sun timer two branches later moves the tilt.
  2. The receive pulse decoder has no tests. handleReceive (~3839–3969) turns pulse durations into bits. It is pure logic, so recorded pulse trains could test it in the native env, the way SomfyFrame is tested.
  3. SomfyShade has a wide interface: 6 publish overloads, 12 p_* setters, public fields, and seven sun/wind fields (timestamps and done flags) that are really one sensor state.
  4. Hidden dependencies. Shade methods use the global somfy for isDirty, commit(), transceiver.config and updateGroupFlags(). This plan leaves them alone. Removing them belongs to the "only loop() owns the shade model" work in KNOWN_ISSUES.md.

Plan

Phase 0: delete the dead code

Done in bbfb23e. sort_asc() was dead too (only called from a dead block) and went with it.

One commit, no behaviour change.

  • Remove every #ifdef USE_NVS block (296 lines), including loadLegacy() and SomfyShade::load(). Also remove useNVS(): it sits outside the blocks, but only the blocks call it.
  • Remove the 16 commented-out blocks (392 lines), mostly old toJSON versions and snprintf emitters.
  • Remove the matching declarations in Somfy.h.

This takes the file to about 4,050 lines before anything moves.

Phase 1: split the file by concern

Done in 0a329ca..505108c, one commit per file. Code moved unchanged, apart from bit_length, which became the shared radioBitLength. The 253 top-level definitions before and after match one for one.

Several commits. Code moves as it is: the classes and Somfy.h's interface stay the same, only the files change. A class's methods may end up in several files by concern. Check each move with the esp32devdbg build and the native tests.

New file Lines Contents
Transceiver.h/.cpp 125 + 949 Transceiver, transceiver_config_t, RadioLock/TxQueueLock and the radio statics, the receive interrupt handler and RX queue, the radio task, the frequency scan, and somfy_frame_t::decodeFrame(somfy_rx_t*) (it needs RadioLock). Somfy.h includes Transceiver.h, so callers don't change.
SomfyRemote.cpp 170 SomfyRemote and SomfyLinkedRemote
SomfyShade.cpp 1,532 shade behaviour (the rest of Somfy.cpp, renamed so history follows it)
ShadeGpio.cpp 148 the GPIO and dry-contact methods
SomfyGroup.cpp 134 SomfyGroup behaviour
ShadeController.cpp 449 SomfyShadeController and SomfyRoom. Not SomfyController.cpp, which would look like a pair with SomfyController.ino.
SomfyMqtt.cpp 412 every publish*/unpublish*/publishDisco
SomfyEvents.cpp 99 every socket emitState/emitCommand
SomfyJson.cpp 235 validateJSON/fromJSON for shades, groups and rooms, and the radio's (see below)

Flash size: what the split taught us

The esp32devdbg app partition is 1,572,864 bytes, and before the split the image used 99.4% of it.

  • At -Og, GCC only caps inlining growth in units over large-unit-insns (10,000 instructions). The big Somfy.cpp was over that, so ArduinoJson's helpers stayed calls. In smaller files they were inlined into every caller: moving the JSON parsing out grew the image by 19 KB, past the limit. --param=large-unit-insns=1000 in [env:esp32devdbg] (4cb676a) treats every file as large. That freed 13 KB, and the split itself then cost nothing.
  • Every file that uses ArduinoJson also gets its own copy of ArduinoJson's debug-assert strings, and its own copies of helpers it inlines. So all JSON parsing lives in SomfyJson.cpp, including the radio's fromJSON, which the plan had kept in Transceiver.cpp. Grouping it saved 2.8 KB. SomfyMqtt.cpp (publishDisco builds a JsonDocument) is the only other new file that uses ArduinoJson.
  • Result: 1,546,797 bytes (98.3%), 16.8 KB less than before phase 0.

For phase 2, keep new modules free of ArduinoJson, and check the flash size of every commit. If space runs out, remove log lines first.

Phase 2: pull out the copied logic

One module per commit, each with native tests. These follow the ShadeMotion pattern: plain C++, no globals, and added to build_src_filter in the native env in platformio.ini.

  1. ShadeSensors (sun/wind). Holds the seven sun/wind fields. Interface:

    • onSensorFrame(sunny, windy, now), called when a sensor frame arrives
    • onSunFlag(), called when the sun flag is turned on
    • check(now, sunFlag), which returns none, extend or retract
    • windBlocksDown(now)

    One shade method turns extend/retract into lift and tilt targets, so all three call sites use the same rule. Move the code with behaviour unchanged first. Then fix the tilt-only bugs from problem 1 in a separate commit, with a test that shows the fix.

  2. stepTargets() in ShadeMotion. The integrated-tilt, tilt-only and lift-only branching, built on stepPosition(), used by both processFrame and processInternalCommand. The 1% step in processWaitingFrame (a held step button) may be deliberate; keep it unless decided otherwise.

  3. PulseDecoder. The bit decoding from handleReceive, as feed(durationUs) returning true when a frame is complete. It must stay in IRAM: mark it IRAM_ATTR and don't call out of IRAM. The interrupt handler keeps only the noise guard, the timing and the push to the RX queue. Tests feed recorded 56-bit and 80-bit pulse trains.

  4. Optional, later.

    • The four arrive-at-target blocks in checkMovement (lift up/down, tilt up/down) are near-copies that could share one helper.
    • The six getNext*Id/getMax*Order functions in the controller are copies of each other.

When each module is created, add its terms to GLOSSARY.md: "sun flag", "sensor frame", "pulse decoder".

Risks

  • IRAM. The decoder runs inside the interrupt and must stay in IRAM. Watch the build for dangerous relocation or IRAM link errors.
  • Flash. See Flash size.
  • Inlining flag. --param=large-unit-insns=1000 changes inlining across the whole debug build. Check on the device that radio RX/TX still work.

Decisions

Decided for phases 0 and 1:

  • Phases ran in the order 0 → 1.
  • Headers: Somfy.h stays the one header, with only Transceiver.h split out.
  • Log tags: every file keeps "Somfy". Files that never log (SomfyMqtt.cpp, SomfyEvents.cpp) have no tag.

Still open for phase 2:

  • Tilt-only sun/wind bugs. Fix them in their own commit right after ShadeSensors is extracted (recommended), or record them in KNOWN_ISSUES.md for now.