feat: add us debt screen #2
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "PMK/btclock_v4:feat/usdebt_screen"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
WIP: feat: add us debt screento feat: add us debt screenPR Reviewer Guide 🔍
(Review updated until commit
d5333a0a79)Here are some key observations to aid the review process:
Floating-point precision
In the "precise mode" fallback path (when
us_debt_exactis nullopt butus_debt_tis set), the code computes*us_debt_t * 1e12and casts touint64_t. Sinceus_debt_t(e.g. 39.2) is not exactly representable in IEEE 754 double, the multiplication may produce a value like 39199999999999.998… which truncates to 39199999999999 instead of the expected 39200000000000. In normal operation this fallback never triggers becauseus_debt_tandus_debt_exactare always populated together inPollOnce(), but if they ever diverge (e.g., partial snapshot merge edge case), the displayed precise number could be wrong by one in the last digit.PR Code Suggestions ✨
No code suggestions found for the PR.
PR Code Suggestions ✨
No code suggestions found for the PR.
Persistent review updated to latest commit
d5333a0a79🤖 Code Review — US Debt screen (
feat/usdebt_screen)Manual review by Claude (read the full head content of the new
us_debtcomponent + the integration points, not just the diff). Overall this is a clean, well-structured addition — solid task lifecycle, good RAII, and a sensible compact/precise display split. A few things worth addressing before merge, ordered by severity.🟠 Medium
1. Dead branch in
FormatUsDebtString—main/screens/panel_texts.cpp#L946-L949The
value_p < 10.0branch and the finalelseproduce the exact same format string, so the< 10.0test is dead. Given the< 1.0branch uses"$%.2fP", the intent was almost certainly"$%.2fP"here too (extra precision for single-digit quadrillions). Either restore the intended%.2for drop the redundant branch.2. Triplicated "dollars + 3-digit grouping" logic. The derivation
plus the
uint64 → 3-char groupsformatting is duplicated across three places that must stay in lockstep:panel_texts.cpp BuildUsDebtmain/screens/us_debt.cpp RenderUsDebtScreen+DebtGroupsscreen_manager.cpp ShouldRender(L708) andRender(L1019)A format tweak or bug-fix now needs editing in three spots — high divergence risk. Suggest two small shared helpers, e.g.
UsDebtDollars(const DataSnapshot&) -> uint64_tandGroupDigits(uint64_t) -> std::vector<std::string>, reused by source/screen/manager.🟡 Low
3. NVS read on every
ShouldRendercall —screen_manager.cpp#L700-L707This opens an NVS handle and reads from flash each time
ShouldRenderruns, whereas the other screen cases just compare cached values. Theus_debt_big_charflag is already read once intoRenderPrefs(rp.us_debt_big_char). Reuse the cached value here to avoid per-tick flash I/O and to guaranteeShouldRenderandRenderagree within the same cycle.4. Float fallback precision —
us_debt.cpp#L96-L100(and the two duplicates)The
*us_debt_t * 1e12fallback is doubly lossy:us_debt_tis already rounded to 0.1T (±~50 billion), and thedouble → uint64cast can truncate (e.g.39.2 * 1e12 → 39199999999999). It's effectively unreachable today becausePollOnce()always setsus_debt_exactalongsideus_debt_t, so this is defensive-only — but if the two ever diverge (e.g. a partial snapshot merge), the precise display would be wrong. When usingus_debt_exactthe path is exact (integer part < 2^53), so the fix is just to make the fallback's lossiness explicit or drop it.5. Task stack size — please verify.
us_debt_source.cpp#L127-L129creates the worker with a 4 KB stack while it runsesp_http_client_perform+ cJSON parsing. If TLS terminates on this task (rather than in the proxy transport), 4 KB is tight and a handshake could overflow it. Worth a quickuxTaskGetStackHighWaterMark()check under a real fetch.✅ Looks good / verified
Stop()setsstop_, joins on thedone_semaphore (≤12 s) and only then clearshub_, so there's no use-after-free / data race onhub_fromPollOnce().FetchContextfrees its buffer via RAII. Nicely done.truncatedflag prevents unbounded growth;body[size] = '\0'stays within thekMaxResponseBytes + 1allocation.static_assert(kAgnosticSlots == 11, …)bumped in lockstep with the slot map — good guard against the documented slot regression.heap_caps_malloc_prefer(kMaxResponseBytes + 1, 2, MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT, MALLOC_CAP_8BIT)call is correct — the2is the count of capability arguments (num), not a caps bitmask. It properly prefers SPIRAM and falls back to internal 8-bit RAM. (Flagging explicitly because an automated pass read this as a "wrong caps" bug — it's a false positive.)🧪 Tests
test_host/test_us_debt_panel_texts.cppcovers the panel-text formatting — good. Note the duplicated render/manager grouping logic (point 2) isn't exercised by those tests; folding it into a shared helper would also make it directly testable.Please check my comments.
@djuri Can you verify the dataSource if this is done correctly?
@ -168,3 +170,3 @@| `mempoolInstance` | string | `"mempool.space"` | Base URL of the mempool.space-compatible instance. | Reboot required. || `mempoolSecure` | bool | `true` | Use HTTPS/WSS when talking to `mempoolInstance`. | Reboot required. || `ceEndpoint` | string | `"ws-staging.btclock.dev"` | Custom-endpoint host when `dataSource=1`. | Reboot required. || `ceEndpoint` | string | `"ws.btclock.dev"` | Custom-endpoint host when `dataSource=2/3`. | Reboot required. |@djuri Can you verify the dataSource if this is done correctly?
@ -19,6 +19,8 @@// slot 12 + 3k : kMarketCap api_id 30 for currencies[k]// slot last : kBlockFeeRate api_id 6//// app_id 100 - 128 are for the debt screens (kDebtSlotBase)@djuri As there are 28 new screens, I have added only this line.
@ -73,3 +72,4 @@// agnostic block (after NwcBalance) so the screen's introduction// didn't shift bitaxe / NWC slot indices. New screens added later// should follow the same pattern."base slot") {@djuri Please test the "didn't shift bitaxe / NWC slot indices" part with this fix.
Review — US Debt screen over the v2 WS
metricchannelVerdict: Request changes. This is a well-structured feature — the rate-frame-ticked-locally model is the right design, the slot/catalog integration is clean, and host-test coverage is solid. The wire protocol aligns 100% with the new Go
metricfeed. But there are two must-fixes before merge, and the debt socket's connection target is wired wrong — confirmed both by static analysis and empirically on hardware (Rev B).Empirical verification (Rev B @ staging,
dataSource=3→ws-staging.btclock.dev)I built this branch, flashed a Rev B, pointed it at staging, and read the live panel texts via
GET /api/status(data[]= the current screen's panels), jumping straight to the debt screen withPOST /api/show/screen {"s":100}:data[]on screen 100['US/DEBT','','','','','','']['US/DEBT','$','3','9.','2','6','T']In both runs the control price screen showed
['BTC/USD','$','6','2','9','3','7'](≈ $62,937) — so the staging core feed was alive the whole time; only debt was silent in A. That isolates the failure precisely to the debt-metric routing.🔴 Blockers
1. The debt-metric socket can never reach the server that serves the feed
main/sources/sources.cppThe
metricfeed exists only onws-staging.btclock.devtoday (GET https://ws.btclock.dev/api/v2/metrics→404). But the debt subscription is hardwired to production via two paths:shares_debt_socket = (data_source == 0)(line ~199): with a custom endpoint (dataSource=3), the core socket that is connected to staging is handed an emptymetric_keyslist and never subscribes todebt:US.data_source != 0) is built withBuildBtclockSourceUri(0, "", false)=wss://ws.btclock.dev/api/v2/ws— hardcoded production, ignoringceEndpoint.So there is no settings/runtime path to point the debt socket at the configured custom endpoint. This is broader than the staging period: even after GA, a user running
dataSource=3against a self-hosted btclock endpoint that serves metrics would still not get debt. Hardware-verified fix (2 lines):Optional refinement: have that
data_source == 1socket honorceEndpointinstead of hardcoding production.2. Host tests fail on CI —
ceEndpointdefault mismatchtest_host/test_settings_api.cppasserts theceEndpointGET default is"ws.btclock.dev", butcomponents/settings/include/settings/schema.hppstill declareskCeEndpoint's default as"ws-staging.btclock.dev"(anddocs/SETTINGS.mdagrees with the schema).GET /api/settingsemits the schema default, so the assertion fails deterministically →btclock_host_testsexits non-zero → CI red. Looks like a rebase leftover (test updated, schema/doc not). Pick one source of truth.🟢 Wire-contract alignment — fully verified
A live probe against
wss://ws-staging.btclock.dev/api/v2/wsreplaying the firmware's exact subscribe frame, plus feeding the real server bytes through the vendored ArduinoJson:Subscribed to debt:USbase/rate/ref/dpencode as float64 / float64 / int64 / int →is<double>()/is<double>()/is<int64_t>()/is<uint8_t>()all accept it.Because the Go encoder statically types
base/rateasfloat64, msgpack always emits0xcbeven for whole values, so theis<double>()guards are safe against integer-encoding drift by construction. No legitimate frame is ever dropped. The protocol side is correct; only the endpoint wiring was wrong.🟠 Medium
main/screens/panel_texts.cpp(FormatCompactDebt). The divisor table floors at1e6('M'), so any debt-in-BTC below 1M renders as a misleading₿0.0XM(e.g. Malta ≈₿0.11M), and the non-big-char path rounds*valuetouint64so a sub-1-BTC value collapses to0. Add a sub-million tier and a host test for a small-country BTC value.debtCurrencytoggle bounces the WebSocket —components/webserver/control_server.cpp. It's in theon_screens_changedtrigger, which postskRebuildScreens→SetSubscriptions→ unconditionalStop()+Start()on the live socket. ButdebtCurrencyonly changes rendering; the subscription set is invariant. Route it through the live re-render /MarkDirtypath instead of a TLS reconnect.data_source != 0user, even with zero debt screens enabled. Largely resolved by the Blocker-1 fix.main/CMakeLists.txtlinksdebt.cpp+ the 29-entry catalog + ~170 lines of formatting unconditionally into the 4 MB image. Rev A sits at ~19 KiB free, so please measure the Rev A build before merge. Feature parity across all variants is the goal — if it regresses past the headroom, reclaim space by trimming the footprint (e.g. a leaner catalog representation, folding the duplicated formatting), not by excluding Rev A from the build.main/sources/sources.cpptruly conflicts on a 3-way merge.mainmoved currency-fetch into a deferredRefreshUpstreamCurrencies(); a naive resolution drops the USD/EUR price subscription (which debt-in-BTC depends on) after the first connect. Rebase manually and add a test that USD/EUR survive the post-connect prune.DataSnapshot::Mergedebt loop has no test —components/data_core/hub.cpp.⚪ Low / nit (selection)
Per-tick
ReadRenderPrefs()(~18 NVS reads) in the debtShouldRenderbranch; the debt screen repaints everyminSecPriceUpds while displayed (the counter ticks continuously — EPD wear);BuildDebtPanelTextsis built 3× per paint;dpis decoded but never consumed; deadstarts_with("screen")branch inIsScreenVisibilityKey;ShouldRender/Renderdebt-key diverge only on the first frame (V8, N=8); untested edges inCurrentDebtValue(now<ref), theFormatCompactDebtunit-carry, and the 8-panel layout. Note: the "replace the US-only setting" comment is inaccurate — the prefs are purely additive, no migration involved.Suggested order before merge
ceEndpointfor thedata_source == 1socket too.ceEndpointdefault mismatch; get host tests green.main(semanticsources.cppconflict) + a test that the debt USD/EUR subscription survives the prune.debtCurrencyfrom the WS bounce.Reviewed by reading the full PR-branch files (not just the diff), an adversarially-verified multi-dimension pass, a live wire probe, and an on-device build/flash confirmation on Rev B.
Re-review — after
9e3c4b9+0438a0c(firmware) and webui#179Verdict: Request changes. Functionally correct and hardware-verified, but the debt catalogue must be data-driven before this merges (Required #1), and one latent regression remains.
✅ Addressed since the last review (
9e3c4b9)shares_debt_socket = data_source != 1; dedicated socket only fordata_source == 1. Verified on Rev B — withdataSource=3→staging the US Debt screen renders live data.ceEndpointdefault mismatchws-staging.btclock.dev, matches schema/doc.debtCurrencyPATCH bounced the WSSetSubscriptions.'K'tier added toFormatCompactDebt.DataSnapshot::Mergedebt loop untestedtest_data_hub.cpp.dpunused, deadstarts_with("screen"), whitespace)🔴 Required before merge
1. Drive the metric catalogue from
/api/v2/metrics/info— don't hardcode it. The Go server is the source of truth and now publishes the full per-metric catalogue:(live on staging, 29 entries). The hardcoded
kDebtCountriestable duplicates that catalogue (key/label/region/currency/dp) in the firmware, which means it silently drifts the moment the server adds, renames, re-denominates, or drops a metric: wrong label or currency, a missing country, or a screen that subscribes to a key the server no longer serves — each only fixable with a coordinated firmware release. The firmware already avoids exactly this class of drift for fiat by fetching/api/v2/currencies; debt metrics should follow the same proven path (FetchAvailableCurrenciesinmain/sources/sources.cpp, deferred + non-blocking). Maintaining two hand-synced copies of the catalogue is the kind of liability that shouldn't ship.Two things to handle in the firmware:
api_idper key, persisted in NVS (first-seen → next free id, not list position — the server list is alphabetical, so a positional id shifts every persisteds<id>Visible/screenOrderentry when a country is inserted). This is the one thing the endpoint can't give you, and it's what keeps persisted screen order/visibility stable across catalogue growth.kBaseAgnosticSlots + kDebtCountries.size(). This touchesslot_count()/KindForSlot/ApiIdForSlot/SlotForApiIdand thestatic_assertdrift-guards, so it wants focused tests (the slot-shift regression those asserts protect against).Re-add the
dpfield while you're there and honor it inFormatCompactDebt. Degrade gracefully when/api/v2/metrics/infois unreachable (empty catalogue → no metric screens, same as today's default-off). Availability is no extra risk: the info endpoint ships on the samefeat/metric-debt-channelbranch as the feed itself. Bonus: this removes the 29 string-literal rows from every image, which also resolves the Rev A partition concern below.2. Fix the debt USD/EUR price drop in
RefreshUpstreamCurrencies. Themainmerge (0438a0c) brought in the deferred currency refresh but didn't extend it for debt.main/sources/sources.cpp:288re-subscribes with the pruned user currency list and does not callAddDebtPriceCurrencies— unlike the liveactCurrenciesPATCH path (main/app/control_command_drain.cpp:211), which does. Thedebt:USmetric sub survives (SetCurrencieskeepsmetric_keys_), but the USD/EUR price sub does not, sodebtCurrency=1(BTC mode) loses its conversion price after the first STA connect whenever a currency is pruned and the user has no USD/EUR inactCurrencies. The comment there even claims it's the "same path as the live actCurrencies PATCH hook" — it isn't. Fix: re-applyAddDebtPriceCurrenciesto the pruned set (fordata_source != 1) before re-subscribing, plus a host test that USD/EUR survive a prune with a debt screen enabled.🟡 Nit
sources.cppmislabeldata_source==1as "Nostr" — it's mempool+Kraken;2is Nostr.✅ End-to-end hardware verification (Rev B)
Flashed
9e3c4b9+ webui#179 to a Rev B againstws-staging:$39.31T(compact) and$ 39 314 436 …(groups,debtBigChar=false).metricchannel is correct and empirically verified.💡 Suggestion (optional)
Tag metric screens in
/api/settingsso the WebUI can group them. With 29 debt screens, the flatscreens[]list ({id,name,enabled,order}) crowds the Status column and the screen picker. Adding a discriminator per entry — e.g."kind":"debt"(or a generic"group":"metric"), sourced from the screen catalogue's existing short-name — lets the WebUI render metric screens in their own collapsible "National debt" section, separate from the standard screens. Small, additive change inBuildGetResponse+static/openapi.{json,yml}+ the WebUIScreentype; backward-compatible.Bottom line: make the catalogue data-driven from
/api/v2/metrics/info(Required #1) and fix theRefreshUpstreamCurrenciesprice drop (Required #2), and this is good to merge. The feature is already functionally correct and hardware-verified; these keep it maintainable.Two findings from hands-on testing of
91cb876on a Rev B🟠 Regression: debt screens are missing from
GET /api/settings(so the WebUI can't list/enable them)On the runtime-catalogue build the debt screens render (
POST /api/show/screen {"s":100}→US/DEBT $ 39 377 485 510 772) but are absent from thescreens[]array inGET /api/settings(11 entries, no id ≥ 100). webui#179 sources the screen list solely from/api/settings(ScreenButtons.svelte:settings.screens.filter((s) => s.enabled); no/api/v2/metrics/infofetch), and only adds the two globaldebtBigChar/debtCurrencytoggles — so with the debt rows missing there is no way to list or enable/disable an individual debt screen from the UI. The two PRs are inconsistent: the WebUI expects debt rows in/api/settings; the firmware doesn't deliver them at runtime.Root cause is ordering.
ScreenKindsCatalog()does includeDebtCatalog(), andinit_control_apibuildsctx.screensfrom it — but that runs at boot, beforeRefreshDebtCatalog()(first STA connect) fetches the catalogue. The post-fetch swap rebuilds the rotation but notctx.screens, so the settings list stays a boot-time snapshot with the catalogue still empty. On9e3c4b9(compile-time catalogue) the rows were present —test_settings_catalogsassertedexpected.at(100) == "US Debt"andkEnumCount == 43— so this is a regression introduced by the runtime refactor.The host test masks it:
test_settings_catalogson91cb876seedsDebtCatalog()(viadebt_catalog_seed) before building the settings response, so CI is green — but on-device the order is reversed (ctx.screensbuilt before the catalogue loads), which is exactly the path that fails.Fix: rebuild
ctx.screensafterRefreshDebtCatalog()installs the catalogue (or build the settings screen list lazily at GET time from the liveScreenKindsCatalog()), and add a host test that follows the real boot order — empty catalogue → build context → install catalogue → assert/api/settingsnow contains the debt rows.(This supersedes my earlier "suggestion #2 — tag metric screens in
/api/settings": they first need to be present at all.)🔎 Crash investigation (Rev B coredump): heap corruption, not the debt feature or the fonts
A Rev B running the debt build captured a coredump:
abort()inside stb_truetype's rasterizer (STBTT_assert(z->direction)instbtt__rasterize_sorted_edges) while rendering'8'at pixel_height 196 on the Moscow Time screen (fontName=notoSans, glyph box 72×107, scale 0.143906).I reproduced that exact rasterization on the host with the repo's own
stb_truetype.h(v1.26, asserts on): the byte-identical input (NotoSans'8'ph 196 → box 72×107, scale 0.143906) renders cleanly, and an exhaustive sweep (both NotoSans cuts × all ASCII × ph 120–240) fires no assert. So the glyph data is benign — the rasterizer was the victim of heap corruption, not the cause. The corruptor is not in that backtrace and isn't the debt code (the crash is on Moscow Time).I reflashed the Rev B with
CONFIG_HEAP_POISONING_COMPREHENSIVE=yand tried to provoke a recurrence (~7 min: WS-bounce via actCurrencies churn ≈115 reconnects, render churn across all screens incl. debt + Moscow Time, API/zap/identify/heap-trace flood). No recurrence — heap stayed healthy, no leak. The corruptor is rare/elusive; a poisoning build + watcher are left running so the next occurrence aborts at the offending block (address + caller PC) rather than as a downstream font assert.This isn't a blocker for this PR (it predates and is unrelated to the debt code), but flagging it here since it surfaced on this build and the simulation conclusively rules out the fonts.
🟠 Full-device hang on hardware — main event loop deadlocks in
esp_websocket_client_stop()(pre-existing WS-teardown bug, surfaced while testing this branch)Reproduced on a Rev B running this branch (
91cb876, comprehensive heap poisoning on). The device "hung": e-paper frozen,POST /api/show/screenaccepted (HTTP 200) but never applied,dataand heap completely static — yet/api/statuskept answering andespUptimekept climbing. So it isn't a crash: the RTOS and the httpd task are alive while the application event loop is dead.Root cause (confirmed via JTAG
thread apply all bt)The
maintask (which runsRunEventLoop) is parked forever here:A
kRebuildScreenscommand (a settings / screen-order / currency change) callsSetSubscriptions(...), which performs a synchronousStop()→SafeShutdownWsClient()→esp_websocket_client_stop()with an unboundedportMAX_DELAYwait forSTOPPED_BIT. Inspecting the client handle being stopped (0x3c21ff34) at the halt point, the struct is fully zeroed (task_handle=0,status_bits=0,state=WEBSOCKET_STATE_UNKNOW,run=false) while the waiter is blocked on a now-stale event-group handle it captured before the zeroing. SoSTOPPED_BITis never delivered, the wait never returns, and the entire UI/render/command loop is wedged.This is pre-existing, not a debt regression
At the merge-base (
26d64f9), the samekRebuildScreenshandler already calledctx.btclock_ws->SetCurrencies(new_currencies), andSetCurrencies→Stop()→SafeShutdownWsClient()→ blockingesp_websocket_client_stop()already existed verbatim (so did theSetBlockFeeDecbounce). This PR's only change to that handler is renamingSetCurrencies(new_currencies)→SetSubscriptions(ws_currencies, metric_keys)(identical blocking semantics) and adding a secondSetSubscriptionscall for a separate debt socket. So the deadlock is reachable onmaintoday via anykRebuildScreens/SetBlockFeeDec; the debt change only slightly widens exposure (two back-to-back blocking teardowns when a dedicated debt socket is configured). The teardown path also has a documented race history — see the comment inws_client_lifecycle.hppitself,bd btclock_v4-28n, and the 2026-05-23 UAF coredump.Why it's invisible and unrecoverable
CONFIG_ESP_TASK_WDT_CHECK_IDLE_TASK_CPU0/1=y); a blocked task yields the CPU, the idle task keeps feeding the watchdog, so nothing trips.POST /api/restartdoes not recover it —HandleRestartenqueueskRestarton the very command queue the hung loop is supposed to drain (control_server.cpp:1257), so the reboot request just sits behind the deadlock. Only a JTAG/USB reset brought the device back.Suggested direction (separate from the debt feature)
portMAX_DELAY) and treat a stop timeout as a recoverable error rather than an infinite park.esp_websocket_client_stop()cannot be invoked on a stale/destroyed handle.Probably best tracked as its own issue against
mainrather than gating this PR. Happy to share the fullthread apply all btdump.Correction / retraction of attribution above.
My earlier comment in this thread (now edited) framed the WS-teardown hang as something this PR introduces "via the debt subscription refresh." That attribution is wrong, and I want it stated explicitly in the thread rather than only as a silent edit.
After checking the history: at the merge-base (
26d64f9) the samekRebuildScreenshandler already calledbtclock_ws->SetCurrencies(new_currencies), andSetCurrencies→Stop()→SafeShutdownWsClient()→ blockingesp_websocket_client_stop()(portMAX_DELAY) already existed verbatim — as did theSetBlockFeeDecbounce. This PR only renames that call toSetSubscriptions(currencies, metric_keys)(identical blocking semantics) and adds a second teardown for a dedicated debt socket.So the deadlock is pre-existing in
mainand reachable today via anykRebuildScreens/SetBlockFeeDec; the debt change does not cause it (it only slightly widens exposure). I'm filing it as a separate issue againstmainso it does not gate this PR. Apologies for the initial misattribution.Review —
3069f58"fix(debt): refresh settings screen catalog after runtime load"Verdict: ✅ correct, thread-safe, tested, and hardware-verified. This resolves the
/api/settingsregression I flagged in [#issuecomment-1810] (debt screens rendered but were absent fromGET /api/settings, so the WebUI couldn't list or toggle them).What the fix does
cfg_.screens_catalogis now refreshable instead of a boot-time snapshot.BuildDeviceContexttakes the catalogue as a parameter;GET/PATCH /api/settingsread a freshScreensCatalogSnapshot().RefreshDebtCatalog()pushes the updated catalogue (with the debt screens) via the newSetScreensCatalog()after the runtime fetch completes — so the debt screens appear in/api/settingsonce the catalogue loads.Strengths
settings_catalog_mu_guards the cross-task access — the httpd worker (GET/PATCH) vs. the refresh on the network task. Everycfg_.screens_catalogaccess now goes through the mutex, andBuildDeviceContextreads the snapshot rather thancfg_directly. This also closes the catalogue-swap-on-network-task nit from the earlier review."settings screens refresh after runtime debt catalogue loads"starts with an empty debt catalogue → asserts screen100is absent → loads the catalogue + rebuilds → asserts100is present ("US Debt"). No more pre-seeding mask, so this test would have caught the original regression. Full host suite green (1159 cases / 21132 assertions).Hardware verification (Rev B, OTA,
dataSource=3→ws-staging)GET /api/settingsnow lists all 29 debt screens (id 100–128:US Debt … SK Debt) withid/name/enabled.US DebtandNL Debtrender live:US/DEBT $ 39 378 540 133 003andNL/DEBT € 583 297 260 870. This closes the firmware↔WebUI inconsistency — webui#179 can now list and toggle them.⚠️ Rev A partition: the debt feature now fits, but with ~0 headroom
I built and USB-flashed this branch to a Rev A (4 MB). It boots and runs, and the debt screens are present in
/api/settings. But it only just fits:For reference,
main(no debt) leaves ~17.5 KB free on Rev A; the debt feature costs ~14 KB there. 3.5 KB of headroom means any further growth overflows Rev A. Since feature-parity across variants is a hard requirement, this is worth addressing now by trimming the feature's own footprint (e.g. the runtime catalogue is already leaner than the old hardcoded table; the display-label strings + per-countryScreenEntrycopies are the next candidates) rather than letting Rev A drift toward an overflow.Re-raising: tag the metric screens (was Suggestion #2 in [#issuecomment-1794])
I marked that suggestion superseded in [#issuecomment-1810] only because the screens weren't present at all yet. Now that they are, it's live again — the flat
screens[]list puts 29 debt rows into the Status column and the screen picker. A small, additive discriminator per entry (e.g."kind":"debt"/"group":"metric", sourced from the catalogue) inBuildGetResponse+static/openapi.{json,yml}+ the WebUIScreentype would let the WebUI render them in their own collapsible "National debt" section. Backward-compatible; optional but it keeps the screen list usable at 29+ entries.Minor (non-blocking)
RefreshDebtCatalog()'s AP-mode early-return path swaps the catalogue but returns beforeSetScreensCatalog(), so in pure-AP/provisioning mode/api/settingswouldn't list the debt screens. Almost certainly fine (you don't configure debt screens from the captive portal), just flagging.Nice fix — the regression is gone and it's verified on both Rev B and Rev A.
🟠 Debt screen alignment: empty panel between the currency symbol and the value (any debt with < 5 digit groups)
Observed on hardware (Rev B + Rev A,
ws-staging). The grouped (non-debtBigChar) debt screen leaves a blank panel between the currency sign and the number whenever the value has fewer than 5 three-digit groups:So
US/EU27(both ~14-digit, 5-group) look fine, but every smaller-debt country (NL and most of the EU set) renders€ ␢ 583 297 …with a hole between the symbol and the first group.Root cause
BuildDebtPanelTexts, the grouped path (main/screens/panel_texts.cpp):The padding for the missing groups is inserted after the symbol, which right-aligns the digits but strands the symbol on the far left. The
debtBigCharpath right above it already does the right thing — it builds[symbol, amount…, unit]and pads at the front (cells.insert(cells.begin(), …)), keeping the symbol attached to the value.Suggested fix — mirror the big-char path (pad before the symbol)
NL then renders
['NL/DEBT', '', '€', '583', '297', '260', '870']—€adjacent to the value, the blank panel moved to the front (where it reads as right-alignment, not a gap). US is unchanged (already fills every slot).Test gap
test_debt_panel_texts.cppcoversUS debtandEU27— both ~14-digit / 5-group, so neither exercises the< group_slotsbranch that has the bug. Worth adding a sub-5-group EUR case (e.g. an NL-sized value) assertingcells[1].empty()andcells[2] == "€"so the alignment is pinned.(Affects only the grouped layout;
debtBigChar=trueis already correct.)View command line instructions
Manual merge helper
Use this merge commit message when completing the merge manually.
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.