Repository navigation
Supervisor integer only - #2
Merged
Merged
Conversation
…side Three changes that turned out to be one: a coding standard needed writing, and writing it found that the component it was mostly about passed through no static analyser at all and ran under no sanitiser. THE SUPERVISOR WAS ANALYSED BY NOTHING. SWR-NFR-25 says code shall pass the project's static analysis configuration. .clang-tidy's HeaderFilterRegex named only firmware/controller, tools/tidy.sh built databases from the controller's two host projects, and tools/tidy-target.sh hardcodes kiln_hal_esp32s3. Tasklist C11 records the same hole for main.cpp and httpd.cpp and never mentioned the supervisor, so the one component whose entire argument is that it can be independently reviewed was the one component clang-tidy had never seen. tidy.sh now drives four databases, 65 translation units. The supervisor's two target-only units need no ARM toolchain: src/main.cpp and board/startup.cpp include nothing but <stdint.h> and the project's own headers, so they are analysed against a synthesised database with clang's thumbv6m-none-eabi target and -ffreestanding, carrying the same vacuous-pass canary tidy-target.sh has. STM32CubeCLT is needed to build the image, not to parse it. It reported 207 findings, all in the supervisor, and the disposition matters more than the number. 46 were macro-to-enum in the core headers and are fixed: every object-like macro in core/ and protocol/ is now constexpr, SUP_C_TO_Q7 among them, which became a constexpr function. 7 were const-correctness in the suites and are fixed. 3 are narrow false positives carrying a NOLINTNEXTLINE with the reason. The remaining ~150 are one class, memory-mapped register access, confined to board/ and src/, and are exempted by two directory-scoped .clang-tidy files on the same grounds kiln_hal_esp32s3 already has one. Nothing is relaxed for core/ or protocol/: the safety function is clean under the unmodified root profile, and that is the half the independence claim rests on. THE SUPERVISOR IS INTEGER-ONLY, closing tasklist R10. The internal unit is q7, 1/128 degC in an int32_t, which is the MAX31856's own LSB, so the hot-junction decode is a shift and nothing else. The wire stays integer tenths and the one scaling step is sup_q7_to_dc, which has its own tests. nm on the linked image finds no __aeabi_f* symbol and no lroundf; text went from 6736 to 3488 bytes, the soft-float helpers having been 2.7 kB of a 6.7 kB image. Size was the least of it. A safety function depended on a library this project does not build, test or qualify, which under 61508-3 is in scope for the argument and under 60730 Annex H is code in the declared software class; the cheapest way to discharge that is not to link it. And every comparison carried a NaN case, correct and subtle and exactly the kind of reasoning a later reader simplifies away. An int32_t has no NaN, so those cases are unrepresentable rather than handled, which is stronger than a passing test. The same reasoning removed the dt sign guard: a uint32_t interval cannot be negative. Two things were hardened while converting rather than translated. Timers saturate via add_ms, because a latch timer that wrapped would fall back below its threshold and un-arm a condition still present, the one arithmetic failure here that would cost protection instead of causing a nuisance trip. And disagreeing takes its magnitude in unsigned arithmetic, so neither a signed overflow nor negating INT32_MIN is reachable even for inputs the decode cannot produce. Both have tests; the NaN tests they replace are gone. --specs=nano.specs stays and its reason changed. The only remaining C library dependency is memcpy and memset, emitted by the compiler for zero-initialised structs rather than written by anyone. Verified by trying -nostdlib, which now fails on exactly those two and nothing else. Supplying them locally wants its own review, so the comment records the dependency instead of a flag changing. THE SANITISER RUNS WERE REPORTING FINDINGS AND PASSING. SWR-TST-20 only ever said the build shall support the sanitisers and the supervisor's did not, so it gained ENABLE_ASAN and a second ctest step. Adding it exposed that UBSan's default is to print a runtime error and continue, leaving the exit code at zero: ctest reports PASSED for a test that triggered real undefined behaviour, and the controller's asan+ubsan leg had been green on those terms over three findings since it was written. Both builds now pass -fno-sanitize-recover=undefined. Those three findings were one class and not defects: test_faults casting 999 to a fault code, test_safety_current casting 200, test_suplink feeding 200 as a wire trip reason, each deliberately, to prove a value arriving from outside is rejected. That is what firmware/controller/test/.clang-tidy already exempts EnumCastOutOfRange for, so UBSan's enum check is excluded in both CMakeLists with the reason written down, separately from the recovery flag, because "fires on a negative test" and "never fail on anything" are different claims. unsigned-integer-overflow is deliberately off too: add_ms and the disagreement magnitude both rely on unsigned wrap being defined. The gate was shown to fail before being trusted, the way the tidy canary was: reintroducing the signed hi - lo that disagreeing avoids produces "signed integer overflow" from test_trip and fails the sanitised step while the plain step still passes. A gate that prints its findings and goes green is worse than no gate, because it looks like one. docs/coding-standard.md is the document v-model-process.md section 5 has always named as an input to implementation and that did not exist. It separates what 60730-1 Annex H asks of the code, which is diagnostics and explicitly no language subset, from what 13849-1 clause 4.6 and 61508-3 table B.1 ask, which is the subset and is why a coding standard exists at all. It names MISRA C++:2023 as the supervisor's target and records that the supervisor compiles clean at C++17, which is the dialect that standard is written against, so adopting it is a tooling purchase and paperwork rather than a rewrite. Nothing here claims MISRA compliance and section 3 says why the word must not be used. Section 10 lists seven gaps that are not met, including the three this commit does not close: no commercial MISRA checker has run, no tool qualification argument exists for the CRC stamper or the compiler, and worst-case stack is still unbounded at build time though -fstack-usage is set. The TC2 ~FAULT input on PB4 is in here too, which was in the working tree and cannot be separated: it changed pin_low to take its port on the same lines of main.cpp that the q7 conversion touched, and splitting it would mean committing an intermediate state that was never built. Verified: supervisor 4 suites and 81 tests pass plain and under ASan+UBSan, controller 25 suites pass in both, clang-tidy clean over all 65 translation units, MC/DC 100 % against its 80 % floor, and the target image builds warning-free at 3488 bytes.
The board, schematic and project file are renamed from kilncontrol to safekiln, matching what the firmware, the image and the documentation have called this product since the repository was reorganised. Three references had to move with it, and they are the ones that would have failed silently rather than loudly: fixture/pinside-baseline.json names the board it is a baseline OF, so a rename without it means `pinside check` has nothing to compare against. fixture/kilncontrol-fixture.json carries an absolute path to the board. It keeps its own filename: that is the fixture's name, not the project's, and renaming an input this project owns is a separate decision from renaming the board. fixture/README.md documents the command a person types. Not touched: fixture/firmware/ and fixture/board/, which pinside generates verbatim and will rewrite from the new name on the next run, and which CI already excludes from the SPDX check for the same reason.
The Cyber Resilience Act requires that a security fix can reach a unit, and SRR-11 recorded that none could without visiting it. OQ-08 and OQ-R4 are now answered, and the answer is a pull path rather than the staged push the question also offered. THE DIRECTION IS THE WHOLE DESIGN. The device checks a signed static manifest at update.bitcrushtesting.com once a day, announces a release on the LOCAL DISPLAY, and installs it only when somebody confirms there. A push path would have cost SEC-00 its absolute form, because an inbound route exists whether or not anybody is standing at the kiln; an outbound GET creates nothing on the LAN to talk to. SWR-WEB-26 and SEC-00 are therefore unchanged by this. What it costs is written down rather than smoothed over. TH-04 is Active again after being closed by the absence of the feature; TH-14 and TH-15 are new; SEC-06 narrows from "no outbound connection" to one, first-party, carrying nothing about the device; SRR-12 and SRR-13 are the new residual risks. SEC-12 carries the goal as three conditions that must hold together. tools/update-manifest.py builds and signs the manifest: an envelope whose payload is the base64 of the exact signed bytes, so a device verifies over those bytes and parses afterwards and canonicalisation is nobody's problem. RSA-3072-PSS-SHA256 from a key it refuses to let be the secure boot key (SRR-12: that one can never be rotated, this one is used every release). It also refuses an unsigned image, an image whose chip id is not this target, a version carrying build metadata and a security release with no advisory, because each of those produces a manifest every device must reject after downloading it. 80 self-test checks. SECURITY.md is the other half and was the most conspicuous gap: a disclosure policy, the Article 13(17) single point of contact, and the Article 14 reporting commitments, which apply from 11 September 2026 and do not wait for the firmware. security.md section 13 inventories what the Act still requires, in two tables: the firmware half, where six rows are evidence rather than work, and the half that is not firmware, whose Owner column is deliberately empty because this document cannot supply one. Three gaps it names had never been recorded: security event logging with an opt-out (SEC-13, SWR-LOG-16), permanent erasure as a function rather than a line in a manual (SEC-14, SWR-CFG-09), and an SBOM from the build (SWR-NFR-28). Two open questions changed character: OQ-S4, the access point that starts without a passphrase, is a secure-default requirement and the only live weakness in code that exists today, and OQ-S3, flash encryption, stops being a trade the project may decline on the sold-units route. Also in here: section 6.1, on why the supervisor's SWD port stays open. The STM32G031 has no secure boot to enable, and RDP Level 2 would close the port the independence argument rests on while an attacker who can reach the board can jumper the permit line more cheaply than reflash anything.
SWR-NFR-28 and the Cyber Resilience Act want a machine-readable SBOM kept for the support period. The question it has to answer under time pressure is "an advisory landed this morning against mbedTLS, did we ship it", and a project that cannot answer it has to assume the worst about every unit it sold. So the one property that matters is that the inventory comes FROM THE BUILD. tools/sbom.py reads build/project_description.json, which names the components that were actually linked, and each component's licence is scanned from the SPDX-License-Identifier tags in its own sources. A curated map in the tool would have been the hand-maintained inventory this exists to avoid, and a component carrying no tag comes out NOASSERTION rather than as a plausible Apache-2.0. CycloneDX 1.6 by default, SPDX 2.3 on request, both from one inventory so they cannot disagree about what shipped. Reproducible: sorted components, a serial derived from the inventory rather than generated, SOURCE_DATE_EPOCH honoured from the tag's commit date, so a diff between two releases shows the product changing rather than the clock. Run against the real ESP-IDF v6.0.1 tree, which found three things no synthetic test would have: ESP-IDF licenses its test_apps under CC0-1.0 and "Unlicense OR CC0-1.0", which made half the components look dual-licensed in a way no shipped byte is. Skipped by pattern now, and what survives is right: FreeRTOS keeps MIT, esp_rom keeps BSD-3-Clause, and wpa_supplicant keeps a CC0 that is genuinely there in fastpbkdf2.c. build_components carries one empty string, which was producing a component called "esp-idf-" with no version and no licence. An SBOM entry for nothing is worse than a missing one, because it looks like data. Two components whose names differ only by an underscore would collapse to one SPDXID and a reader would silently lose one. That is an error now. 148 components on a real build, four of them untagged upstream and pointed at their licence files instead of guessed at. The release workflow publishes both formats as release assets rather than CI artifacts that expire, and generates them inside the IDF container, which is the only place a licence can be read.
…e fixed yet Both red jobs trace to the firmware/controller move, or look like they do. THE ESP32S3 TIDY JOB died with exit 127 on "tools/tidy-target.sh: No such file or directory". The step ran `cd "$GITHUB_WORKSPACE"` first, and that variable is not exported into the IDF container, so `cd ""` quietly succeeded, left the shell in firmware/controller, and the next line looked for the script there. It is `cd ../..` now, relative to the directory the action actually runs in, which does not depend on an environment this container may or may not carry. tools/run-qemu.sh had the same class of bug from the same move and is fixed with it. THE QEMU SMOKE TEST IS NOT FIXED, and this commit does not pretend otherwise. Its log ends at QEMU's own launch banner and carries nothing from the target, so all six assertions fail for one reason and none of those reasons is the firmware. The obvious suspects were output buffering and the missing tty, and both are wrong: the same commands on a developer machine with the same IDF and the same QEMU produce 20 kB of ROM output and telemetry, and still do when the run is killed with SIGKILL and when stdin is /dev/null. Rather than ship a guess dressed up as a fix, the step now reports what the next run needs: which qemu-system-xtensa it found, what version it claims, the exit status of idf.py qemu (previously swallowed by `|| true`, so "exited at once" and "ran three minutes in silence" were indistinguishable), and the byte count of the log. The assertions stay as they are, because a smoke test that is quietly skipped is worse than one that fails loudly. Tasklist C14. The tasklist also loses R10, R11 and R12, which are done: the file's own rule is that an item leaves it when it is finished and the commit that did it is the record.
Taking a screenshot for the README is how these were found, which is the argument for having one. POWER AND ENERGY WERE OUTSIDE THE GRID. index.html gave the two tiles class="tile" and app.css has no .tile rule at all, so they rendered as loose text over the row instead of as the readouts they are. THE HINT UNDER THE CHART READ "undefined-phase, undefined CTs fitted". refreshPower() asked /api/current for `phases`, `channels` and `per_phase`, and the API has never emitted any of them: three-phase went out of scope with OQ-06 and the client kept asking. The API now answers what the interface has to say, because power is the measured current times the configured supply voltage and a reader whose supply is 240 V needs to see which figure produced the kilowatts: "1 CT fitted. Power assumes 230 V AC." One channel is structural rather than configurable, so it is stated rather than counted. TWO NOTICES SAID THE SAME THING ON THE DASHBOARD. The programs notice sat outside section#view-programs, so it rendered under every view. It is inside its section now, which removes it from the dashboard and puts it where it was meant to be. Also: the brand is the product's name rather than KILNCONTROL, operator-facing text carries no em dashes, and no requirement identifiers appear in the interface. SWR-WEB-26 is why the notices exist; it is not something to show somebody firing a kiln. The harness gained --fire. The interface is read-only by design, so a firing cannot be started from a browser and an idle dashboard shows almost nothing of what host/webhost is for; it starts a built-in example through the same kiln_app_start the local display calls, so what the picture shows is a real run of the real logic. The README says so under the screenshot rather than letting it look like a photograph of a kiln.
The access point is removed rather than regulated. It came up whenever the station could not join, served a page whose entire purpose was to accept credentials, and could start open because nothing required net.ap_pass to be set. That was TH-11, SRR-08 and OQ-S4, and OQ-S4 was the only item on the Cyber Resilience Act list that was a live weakness in code that existed rather than something unbuilt or on paper. A rule about a surface is worth less than not having the surface. SWR-NET-02 and SWR-NET-05 are withdrawn, SWR-NET-11 and SWR-NET-12 replace them, and the device now has exactly one radio role: station. net.ap_ssid, net.ap_pass and net.wifi_mode leave the configuration schema, there being no mode left to choose. WHAT REPLACES IT NEEDS SOMEBODY STANDING AT THE KILN. The network screen starts a scan; the list shows what was found, strongest first, with a mark for the ones that want a key; selecting one opens a character-by-character passphrase entry on the rotary encoder. The same argument as SWR-WEB-26: the display and the encoder are already fitted, the operator is already there, and what can be done there cannot be done by a passer-by at all. Details that are requirements rather than implementation, because each one is a network the kiln could otherwise not join and there is no fallback to correct it from: Every printable ASCII character is reachable, and a test walks all 95 of them. Delete and connect sit at the END of the set: at the start they stand between the operator and the letters on every single character, at the end they are one turn backwards from 'a'. A passphrase shorter than eight characters is not offered to the radio. WPA cannot be shorter, and spending a join attempt to discover that costs thirty seconds and tells the operator nothing. Abandoning entry discards what was typed rather than leaving half a passphrase for the next attempt. Credentials are persisted BEFORE the join, so a device that joins and then loses power comes back to the same network rather than to a display nobody is standing at. "Scanning" and "none found" render differently, which is the whole of what an operator needs during the two seconds a scan takes. Nine host tests over the screens, which are still a pure function of (view, event) and need no radio. The scan is esp_wifi_scan_* behind port_net, async so the display keeps drawing, with WIFI_EVENT_SCAN_DONE as the only thing that knows it finished. Builds for esp32s3 at 246 kB. Unproven on hardware (tasklist O4), and that matters more than it would have before: this is now the only way credentials reach the device. The screens are also English-only, which SWR-NFR-23 does not allow and J has to close.
… is written against Section 4 of the coding standard has said "the supervisor is C++17" for as long as it has existed, and the build said 20. MISRA C++:2023 is written against C++17, and naming one standard while compiling at another is the kind of discrepancy an assessor finds before the project does. It cost nothing, twice over. The four host suites pass unchanged, and the linked image is BYTE-IDENTICAL to the one C++20 produced: 3 488 bytes, program memory digest 0x6b9284a1 either way. The language level moved and not one instruction did. That is less surprising than it sounds. The supervisor uses constexpr, nullptr, static_assert, noexcept and three attributes, and nothing newer than C++11: no auto, no enum class, no templates, no override, no type aliases. It compiles clean at 17, at 14 and at 11. Seventeen is chosen to match the subset rather than to reach for anything in it. SWA-20's reason for C++20, the 152 designated initialisers naming the controller's safety defaults, is a controller concern and the controller keeps C++20. The suites' own CMakeLists moves with the target's, so the code the tests translate is the code that ships. Closes item 2 of the coding standard's open-items table.
Both workflows were invalid and every run failed in zero seconds with "this run likely failed because of a workflow file issue". My doing: the SBOM summary steps embedded `python3 - <<'PY'` and a heredoc body starts at column zero, which ends the block scalar the step is written in and takes the rest of the file with it. release.yml carries a comment warning about this exact trap, one step below where I did it, and the warning is about the other direction: a heredoc inside a block scalar carries the block's indentation into its output. Both halves of the trap are real and the file now avoids it entirely. jq instead, which is on every GitHub runner and stays indented where the YAML needs it. Verified against a real 148-component CycloneDX document rather than against my expectation of one, and both files now parse. Validating the YAML before pushing would have cost nothing and caught it: it is one `yaml.safe_load` call.
…ave it
V-model level 5 has said "not written" since the process document existed. It
is now generated instead, by tools/udd.sh out of doxygen comments in the
supervisor's own headers, and the reason it is generated is the reason it was
never written: a unit design kept in a separate file is a second description of
the code, and when the two disagree it is found by whoever is auditing rather
than by whoever is editing.
WHAT AN ASSESSOR READS A UNIT DESIGN FOR is narrower than "documentation" and
is always the same five questions, so there is one alias per question and each
is answered in a fixed place per unit rather than somewhere in a paragraph:
\implements the requirement that makes the unit exist
\param/\rangeof what may be passed, in units and bounds, not types alone
\errorbehaviour what happens when the input is wrong, which on a safety
function is most of the specification rather than an edge case
\rationale why it is this way and not the obvious way
\verifiedby the test that covers it
\rationale is the one that gets left out and the first one an assessor asks
about, because a decision with no recorded reason has to be re-derived by
whoever reviews it, and re-derivation is where a safety argument quietly
changes. \implements, \derivedfrom and \verifiedby are \xrefitem aliases, so
each becomes a cross-referenced index page: "which unit realises SWR-SAF-37"
and "what covers sup_step" are answerable from the document rather than from a
table somebody maintains.
trip.h is complete, which is the whole trip logic: four functions and the six
thresholds, including SUP_OVERTEMP with why 1350 sits 50 degC above the
controller's ceiling rather than on top of it.
TWO GATES, AND THE SECOND ONE IS THE POINT. doxygen's WARN_AS_ERROR fails the
build on an undocumented parameter or a dangling reference. But doxygen says
NOTHING about a file carrying no comment blocks at all: with EXTRACT_ALL off it
simply omits it, so a clean run can mean "nothing is documented" as easily as
"everything is". That is the vacuous pass this project refuses for coverage,
for MC/DC and for the target analysis, and the refusal should not stop at
documentation. tools/udd-coverage.py counts public functions and constants from
the HEADERS, independently of the document, and prints the number every time:
10 of 76 today, trip.h complete and three headers at zero. Tasklist R13.
Found while writing it, and worth recording because it is the trap in this
format: a /** block opened inside the SPDX comment nests the comment, doxygen
parses nothing after it, and every alias in the file then reports as
"unexpanded" with no hint that the cause is 300 lines above.
EXTRACT_ALL stays NO for the same reason: with it on, every undocumented
function appears with an empty description and the document looks complete
while saying nothing.
Debris from running doxygen with a configuration piped in while the working directory was firmware/supervisor: it wrote a LaTeX tree there for a throwaway probe header, and `git add -A` took it along with the unit design work. Nothing referenced it and nothing generated it on purpose. The directory is ignored now so the next stray run cannot repeat it.
THE SSID TRUNCATION. GCC rejects `snprintf(name, 19, "%s", ssid)` where ssid is 33 bytes, with -Wformat-truncation, and it is right to: truncation written that way looks accidental. It is not, the display line fits 18 characters, so the bound is explicit now as "%.*s" with the width passed. Apple clang does not warn, which is why it reached CI; gcc-16 with the project's own flags now compiles the file clean locally. Three jobs were red on this one line: both host suites and the coverage gate. TOOLS/TIDY-TARGET.SH LOOKED FOR THE BUILD WHERE IT USED TO BE. The previous commit fixed the job's path to the script; the script then ran for the first time since the firmware/controller move and reported "no firmware/build/compile_commands.json -- run idf.py build first", which reads as a missing build and was a stale path. Same move, one level deeper, and only visible once the first fix let the script run at all. Confirmed on the run that found them: the esp32s3 analysis job now reaches the script and executes it, and the new unit-design job passes. Firmware build, clang-tidy, supervisor tests, StrictDoc and layering stayed green throughout. QEMU remains red and remains undiagnosed (C14); it now prints which binary it found and what idf.py qemu exited with, which is what the next run has to say.
…ings
THE QEMU SMOKE TEST WAS NEVER RUNNING QEMU. The instrumentation added two
commits ago answered it on the first run:
--- qemu: NOT ON PATH ---
qemu-system-xtensa: command not found
--- idf.py qemu exited 127 after 71 bytes ---
Exit 127, and `|| true` had been swallowing it since the job was written. The
log was not truncated, not buffered and not killed too early, which were the
three theories; there was no emulator. `idf_tools.py install qemu-xtensa`
installs it and `export.sh` does not put it on PATH in this image, so the step
now finds the binary itself and fails loudly if the install produced none,
rather than running a smoke test with nothing to smoke.
Worth keeping as a lesson: the step was fixed by making it say what it was
doing, not by guessing better. Three plausible explanations had been eliminated
by experiment on a developer machine, and all three were irrelevant.
THE ESP32S3 ANALYSIS JOB FOUND TWO THINGS ON ITS FIRST REAL RUN. It had been
dying before clang-tidy started, first on a path to the script and then on a
path inside it, so these had never been seen:
hal_net.cpp:150, mine from the WiFi scan: reinterpret_cast from the driver's
uint8_t[33] SSID to char*. memcpy instead. The octets are the same octets and
copying needs no claim about aliasing.
hal_prod.cpp:48, not mine and older: a const-correctness finding on an
esp_err_t. One word.
That is the argument for fixing a broken gate rather than deleting it: a job
that cannot run reports nothing and looks like a job that found nothing.
The QEMU job prints exit 127 and "qemu-system-xtensa: command not found". Recording what was eliminated as well as what was found: buffering, the missing tty and the SIGKILL were all tested and all irrelevant, and the step was diagnosed by making it report rather than by guessing better. The PATH search added for it still does not find the binary, so the next three things to try are written down in the order they should be tried.
…teeth 76 of 76 public functions and constants in core/ and protocol/, with no exemption of any kind in either. selfcheck.h, max31856.h and sup_proto.h join trip.h, and the struct fields go with them: sup_input_t's every field is a parameter with a range, which is what level 5 asks for and what an assessor reads it for. THE GATE WAS PASSING VACUOUSLY AND NOW DOES NOT. Four combinations of two settings, established by experiment because the interaction is not obvious: EXTRACT_ALL HIDE_UNDOC_MEMBERS an undocumented function NO YES SILENT, the member is simply dropped NO NO WARNS <-- now set YES YES silent, doxygen disables the warning YES NO silent, same reason EXTRACT_ALL = YES is the trap: it reads as "document everything" and switches WARN_IF_UNDOCUMENTED off, so the document fills with empty entries and the build stays green. For a safety target that is the worst of the four. HIDE_UNDOC_MEMBERS = NO costs something visible: an undocumented member appears in the output as a bare declaration. That is the right cost. A gap you can see is a gap somebody fixes. WHAT NEITHER SETTING CATCHES, which is why tools/udd-coverage.py is not decoration: doxygen cannot see a file with no \file block at all, members included, and warns about nothing. Two of these four headers were in that state while the build was green. The script checks for the block, counts declarations from the HEADERS independently of doxygen's own output, and --require-complete fails the build on any gap. It earned that place twice in one sitting. The second time: an EXCLUDE_SYMBOLS pattern meant to exempt board-layer register addresses, SUP_RCC_*, reached into core/ and silently removed selfcheck.h's five clock-integrity constants from the document. Nothing warned, because an excluded symbol is not an undocumented one. The count noticed, and the pattern is SUP_RCC_APBENR* and SUP_RCC_IOPENR* now. ONE EXEMPTION, at the hardware boundary, stated where it is taken: 44 register addresses and pin numbers in board/. The files stay in the document with their pin table and their compile-time proofs that no two pins share a bit. What is exempted is an entry per address, and the provenance an assessor needs is stated once in the file blocks: CubeMX's MCU database for the pinout, the CMSIS-SVD for the register bits. The permit polarities are NOT exempted and are documented individually, because a polarity is the difference between permitting heat and refusing it. The image is unchanged: 3 488 bytes, digest 0x6b9284a1, same as before the first comment was written.
Converting SUP_RAM_PATTERNS' comment into a doxygen block put a second comment between the suppression and the declaration, so bugprone-dynamic-static- initializers fired again and the clang-tidy job went red. The suppression is back on the block's own closing line, which is where the original had it and the only place it works. The documentation is unchanged at 76 of 76, and the two gates disagreeing about the same line is a fair description of what a comment has to satisfy here.
Third and final cause, and it was mine: the step was run by the IDF container
action as bash -c with the WHOLE command block inside single quotes, so the
apostrophe-free rule applies to every character of it, comments included. My
diagnostic block contained echo with a quoted string. Bash exited 2 on a parse
error before running a single line, and a parse error produces no output at
all, which reads exactly like the empty log the job had been failing with the
week before for an unrelated reason.
So the block is now four lines with no quoting hazard, and the work lives in
tools/qemu-smoke.sh. That is the same arrangement tidy.sh and tidy-target.sh
already have, and the reason is the same one: a script in the repository can be
read, reviewed and RUN, and this one now has been.
Verified end to end on a developer machine before pushing, which is what the
previous three attempts lacked:
qemu: .../qemu-xtensa/esp_develop_9.2.2_20250817/.../qemu-system-xtensa
QEMU emulator version 9.2.2
log: firmware/controller/qemu.log (77814 bytes)
ok: the composition root ran
ok: the simulated plant was bound
ok: a program was accepted and started
ok: the run controller reached Running
ok: the safety supervisor granted heat authority
ok: the simulated kiln passed 100 degC
ok: no fault latched
The script also fixes the two earlier causes rather than inheriting them. It
finds qemu-system-xtensa three ways, cheapest first, because export.sh does not
put it on PATH in the container image and that was cause two. It resolves
idf.py as a command or as the script under IDF_PATH, because on a developer
machine it is a shell alias and an alias does not survive into a script, and it
uses the interpreter from IDF_PYTHON_ENV_PATH because IDF refuses a build
directory configured with a different one. --log asserts against a recorded log
without running anything, which is how the assertions were tested against a
real capture from the earlier investigation.
Cause one, for the record, was tools/tidy-target.sh in a neighbouring job. The
three had nothing to do with each other and each hid the next.
tools/qemu-smoke.sh leaves qemu.log in firmware/controller rather than in a build directory, so git add -A took 77 kB of serial output along with the fix. It is a CI artifact and a local scratch file.
main gained 86d9a43 "feat: the supervisor goes integer-only" while this branch was built on c767f2e of the same name. They are the same change committed twice: identical parent 7b8c27b and an empty diff between them, so every conflict the default strategy reported was that change meeting itself. Resolved with -s ours, which is provably the right answer here rather than the convenient one: main has exactly one commit beyond the fork point, that commit is content-identical to one this branch already has, and the merged tree is therefore byte-for-byte the pre-merge tree. Verified rather than asserted.
Status is a third of its length and the component table is gone. The table had become a second tasklist maintained in a worse place: tasklist.md says what is done and what blocks a release, it is kept current, and two lists of the same thing disagree. Status now carries the numbers and points there. The numbers were stale in both directions: 398 host tests is 433, and the supervisor's 81 were not counted at all. "The firmware is C++20" has not been true since the supervisor moved to C++17 to match the standard it targets. Four things were not merely stale but wrong by design, which is worse in a README because it is what somebody reads before the documents: A firing-curve EDITOR on the web interface, two lines below the sentence saying nothing over the network can change anything. Programs are authored at the kiln. "No cloud", when the device now makes a daily outbound call to check for a firmware update. It is one connection, it carries nothing about the device and it can be turned off, and saying that is cheaper than being caught by it. WiFi "with access-point fallback". There is no access point; the network is chosen and its passphrase typed at the display. The same row claimed kiln.local over mDNS, which is specified and not implemented. firmware/controller described as "not yet implemented", which stopped being true some time ago, in a layout that did not mention the supervisor, the tools, the web assets or the fixture. Also: the thermocouple row now says the supervisor reads its own couples on its own buses, which is the whole point of SWA-22 and was invisible in a row that counted two front ends; and the CI paragraph lists the eleven jobs rather than the five it knew about.
A release is the moment somebody asks what this firmware is and what was verified about it. Until now the answer lived in CI artifacts, which expire in ninety days, and in documents you had to build yourself from a checkout you had to get right. Both are now generated from the sources in the tag and published beside the binary, so they describe the image next to them rather than whatever main looked like afterwards: safekiln-<tag>-requirements.zip, StrictDoc over the user, system, software, architecture, safety and security documents, HTML and JSON. The JSON matters as much as the HTML: it is the machine-readable traceability, which is what a tool asking "is every SWR- realised by something" reads. `strictdoc export` fails on a reference to a requirement that does not exist, so the fact that it built is itself part of what the release asserts. safekiln-<tag>-unit-design.zip, V-model level 5 for the independent safety supervisor, generated from its headers: 76 of 76 public functions and constants, each with the requirement it implements, its parameter ranges, its error behaviour, its design rationale and the test that covers it. Documentation warnings are errors, so a gap fails the build rather than appearing as a blank entry. Both commands are proven rather than hopeful: the StrictDoc one is what the requirements job already runs green, and tools/udd.sh runs on a developer machine. The zip path arithmetic was checked locally, which is a sentence worth writing after the week this workflow has had.
The release failed at the assembly step on a missing safekiln-merged.bin. The build step had succeeded, and the merge had written build/build/ because -o resolves against the build directory rather than the project: 'Running merge-bin in directory .../firmware/controller/build' is in the log saying so. Dropping the prefix puts it where every following step already looks. Reproduced and verified locally before pushing rather than by tagging again and watching.
The unit design now draws its own state machine and its own program-sequence graph, from \dot blocks in the headers beside the code they describe. Being in the source is the point: a picture in another document drifts from the code and the drift is found by whoever is auditing. THE TRIP LOGIC'S STATE MACHINE, on sup_step. Four states rather than the two the ASCII sketch had: WAITING does not latch, because "the sensor never got going" and "the sensor was working and stopped" are different and only the second is a fault for an operator to acknowledge; and REVOKED is reachable from either working state and has no edge out of it within a power cycle, which is what SWR-SAF-36 means by unclearable. THE PROGRAM-SEQUENCE GRAPH, on sup_flow_stage_t. READ, STEP, PERMIT, REPORT, and the dashed edges that are the whole reason the measure exists: a cycle that finishes on time having skipped the stage that drives the permit line is something a watchdog cannot see. \dot rather than PlantUML, and the reason is UR-CON-04 rather than taste. Doxygen supports both, plus mscgen and Dia. PlantUML would give strict UML notation and would cost a Java runtime and a jar to vendor; graphviz is already installed beside doxygen for the call graphs, and a state machine in dot reads exactly as well. Doxygen's own automatic UML, UML_LOOK, would have drawn nothing at all here: the supervisor has no classes, no inheritance and no templates. Drawing them found a wrong edge in my own first version: "complete" had an outgoing edge to "broken", which is not a transition, because complete is a verdict rather than a state you leave. The missing-stage case belongs on REPORT. That is the argument for pictures in a unit design, and it arrived within ten minutes of there being one. Graphviz has stopped being optional and tools/udd.sh now says so in one sentence rather than leaving it to a documentation warning. CI and the release workflow already install it.
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.
No description provided.