Tzlindra more data fields#115
Merged
Merged
Conversation
The SessionController computed torque and power but only showed them on
the LCD; the host never saw them. Add session_controller_output_data
{timestamp, torque, power} to the wire schema (protocol v3 -> v4, so a
mismatched pair refuses the handshake instead of missing the fields),
carry it through a new circular buffer to the USB task, and stream it
during a session like the sensor buffers -- including the stale-backlog
skip on session entry and a sample in the MockMessages debug path.
Host side decodes it into a SessionControllerSample, shows Torque (N.m)
and Power (W) on the Home telemetry panel, and logs torque/power columns
in the telemetry CSV.
Also: feed the encoder's measured angular_acceleration into the torque
formula (was a hardcoded 0 marked TO UPDATE, so torque lacked its I*a
term), and fix the USB controller constructing the optical-encoder and
BPM buffer readers with each other's sizes (harmless only while both
are 100).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
MOMENT_OF_INERTIA_KG_M2 defaults to 0 until the rotating assembly is actually measured -- with angular acceleration now feeding the torque calculation, the placeholder 1.0 was actively distorting it. The sysconfig min drops to 0 to match (1e-9 would have rejected the new default at the firmware and flagged it out of range in the host UI). GEAR_RATIO is a new compile-time config.h setting (deliberately neither sysconfig nor streamed; the schema records why). The app reads it from its own copy of the firmware config -- saved override if present, else config.h -- via a new SysConfigViewModel.CompileTimeValue accessor and change event, and the Home telemetry panel now shows angular velocity and torque both as sensed and multiplied by the ratio, plus the ratio itself. Falls back to 1.0 (direct drive) if the headers can't be read, so the geared rows degrade to the sensor values. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
New sidebar page with one scrolling strip chart per telemetry channel -- angular velocity (sensed and geared), angular acceleration, force, torque (sensed and geared), power, BPM duty cycle -- each toggleable, with angular velocity / torque / power on by default. Channels record whether shown or not, so a strip toggled on mid-session arrives with its history. Each strip is its own small multiple: channels carry different units, so there is deliberately no shared (or dual) y-axis; only the 30s time window lines up. The window anchors to the newest sample rather than the wall clock, so a finished run freezes on screen instead of scrolling away; a new session start clears it. Dyno.Core gains TimeSeriesBuffer (ring of timed samples) and Envelope (per-pixel-column min/max decimation, so spikes survive kHz-rate data), both unit-tested. Series colors are the dataviz palette's dark-mode categorical slots validated against the app's card surface; strip titles carry identity so color never does it alone. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The fixed strip-per-channel list becomes a list of graphs the user builds: Add graph creates one (defaulting to the first channel not already shown), each graph picks its channel from a dropdown in its header, and a remove button drops it. Channels and their history are independent of graphs, so switching a graph's channel -- or adding a duplicate -- arrives with recorded history, and removing a graph loses nothing. Sizes are relative via a drag grip along each graph's bottom edge (same affordance as the event log's resize grip), clamped 80-600 px in a scrolling list. Page starts with the old defaults (angular velocity, torque, power) so it isn't blank. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The resyncs that reappeared mid-session were host-side: TelemetryLogger wrote and flushed the CSV per row (~700/s) on the DeviceClient read thread, and as that file grew the flush cost did too, until a stall outlasted the OS serial buffer (4 KB ≈ 250 ms at stream rate) and bytes were silently dropped mid-record. The device's side of the same moment was elevated ping RTTs — its TX retries absorbing the backpressure — with no dropped-batch warnings, which is what pointed here. TelemetryLogWorker now owns the file on its own thread: the read loop hands each message to a bounded queue in constant time, rows are flushed every 250 ms instead of per row, and if the disk ever falls a full queue behind, rows are dropped and counted (reported in the event log) — a complete CSV is never worth a lossy stream. The serial port also gets a 1 MB read buffer (~60 s of slack) in place of the 4 KB default; Linux ignores the setting harmlessly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The stream was the last unframed thing on the wire: bare header+payload records whose only defense against lost bytes was guessing which 12-byte windows looked implausible. Give it the same envelope the host->device direction always had -- [SOF][header][payload][crc16] -- so a cut-off or corrupted record is *detected* (CRC fails, frame skipped, loss reported with the skipped bytes) instead of inferred, and realignment is a scan to the next SOF. Costs 4 bytes per record (~15%); protocol v4 -> v5 so mismatched pairs refuse the handshake rather than misread each other. A bare null terminator was considered and rejected: payloads are raw binary and legitimately full of zeros (a stationary encoder's samples are almost nothing else), so no delimiter can be trusted without byte-stuffing -- whereas SOF+CRC reuses code both sides already have. Also fix the drop-warning false positive that fired 9 ms into a fresh connection: give-up drops are now only counted while a host is acked, and the tally clears on host detach -- announcements discarded while nobody was listening are not lost telemetry, and reporting them to the next host blamed a saturation that never happened. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A graph now holds any set of channels, toggled via header chips that double as its legend (swatch + name, so no line is identified by color alone). Channels carry different units, so there is still no shared or dual y-axis: every series is drawn normalized to its own effective range -- the fixed min/max from Settings > Plots when autoscale is off (clipping outside it), or its live per-frame autoscale fit -- so shapes and timing compare across units. Mixed modes on one graph work. The gridline numbers can only belong to one channel, so each graph gains a "Y axis" picker choosing which shown channel's range labels the axis; it follows the toggles and never points at a hidden channel. The plot control takes its series through a small IPlotSeries interface and re-reads the list every frame (the 30 fps anchor tick), so chip toggles, Settings applies, and axis switches all show up immediately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The scrolling list of fixed-height cards becomes a near-square grid that owns the whole page and reshapes with the graph count: 1 fills it, 2 stack (a time-series strip wants width more than height), 3-4 make 2x2, 5-6 make 3x2, and so on. Every gap between rows and columns is a GridSplitter -- invisible until hovered, accent-lit like the app's other handles -- so dragging a gap trades space between the adjacent row or column. Star-sized tracks with minimums keep a cell from collapsing; proportions reset when the count changes, since the shape they described no longer exists. The per-graph pixel grip and PlotHeight go away with the list. Cells at half-page width can't afford the always-visible chip row (it wrapped into a column that ate the card, leaving no plot), so the header now shows only the shown channels as legend chips and moves membership editing into a compact Channels dropdown of checkboxes. The grid itself is built in code-behind from a keyed DataTemplate: its shape and the splitters between cells are data-dependent in a way a static panel can't express. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…erased at boot Three firmware bugs, found chasing two empty plot channels. BPM produced no telemetry at all, in either mode. Run()'s command loop read the SessionController queue with osWaitForever inside a while, so it re-blocked on every iteration and never fell through -- the only WriteElementAndIncrementIndex in the file was unreachable. The manual path compounded it: START_PWM applied the duty cycle to the timer (so the knob did move the hardware) but recorded no sample, and SessionController only sends on a change, so even a reachable change-only path would have left the readout stale between adjustments. Restructured around a non-blocking drain: commands are polled with timeout 0, PID mode follows the controller's latest value, the PWM state is reconciled once per pass, and a sample is recorded every pass -- a continuous series is what the host's time-series plot needs. SetDutyCycle now returns the clamped value so telemetry reports what is driven rather than what was asked, and raw_value (wire padding) is zeroed instead of left as stack garbage. Run() suspends rather than returns on an unrecoverable PWM failure, since falling out of it would fall off the end of the thread function. Separately, CircularBufferWriter's constructor zeroed the write index it was handed. Harmless for the single-writer sensor buffers, but the task-error buffer is shared: seven tasks each construct a writer over one global index, so every task that started rewound the log to 0 and discarded what earlier tasks had logged. Boot-time errors were reliably erased before a host could ever handshake and read them -- which is why a suspended-at-init sensor task reported nothing. The indices are statically zero-initialized in circular_buffers.c already, so the reset was redundant even where it was harmless. Host tests for the buffer pair, with no-op FreeRTOS critical-section stubs. Verified against the bug: reintroducing the reset fails both wipe tests. One test documents rather than fixes a known limit -- with two indices and no count, a buffer filled to exactly capacity is indistinguishable from empty. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
EffectiveRange returned a series' configured Settings range before checking whether the series had any samples, so a channel that never streamed still got a fully drawn, labeled y-axis with gridlines whenever autoscale was off. With another channel on the same graph the "no data yet" message was suppressed too (drewAnything was already true), leaving a silent channel looking exactly like a plotted one -- flat, or off-range, or hidden under another line. Force reads as plotted while its sensor is dead. Presence is now tested first, and gates everything the series contributes: no line and no axis. The test is O(1) -- buffer times are non-decreasing, so the newest sample landing at or after the window start is the whole question. The axis picker falls back to a series that does have data rather than leaving the graph gridless when it points at a silent channel. The pick itself is left alone: it is the user's standing choice, and should take effect again by itself when that channel starts streaming. Legend chips mark a channel with no samples in the window, driven by the same predicate the renderer uses so the legend can never claim data the strip is not drawing. Absent is now distinguishable from present-but-invisible, which it was not before at all. Tests cover the shortcut agreeing with real windowing across the ring's wrap, the empty buffer, a channel going absent the instant its last sample scrolls off, and Clear() on session start. Verified in the app under Xvfb: a silent Force channel draws no line and no axis, and with the axis deliberately pointed at it the grid falls back to the channel that has data instead of drawing Force's phantom 0-250 scale. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
TIM2 backs get_timestamp(), and its prescaler of 400 put the counter at 500 kHz (2 us per tick) rather than the intended 1 us: the APB1 timer clock is 200 MHz (HSE 25 -> SYSCLK 400 -> HCLK 200 -> PCLK1 100, doubled), so 200 is the divisor that lands on 1 MHz. 400-1 is TIM13's value; 200-1 is what TIM2 wanted. Changed in the .ioc as well as main.c, so a CubeMX regeneration keeps it rather than quietly restoring 400. Nothing needed adjusting alongside it. The only consumer of the rate is the optical encoder, which reads get_timestamp_scale() at runtime and divides its tick deltas by it, so its velocity and acceleration follow the new rate on their own. No other code assumes a scale. Documented the derivation and, more usefully, the consequence: at 1 MHz the 32-bit counter wraps every ~71.6 minutes, which anything measuring across that boundary has to unwrap. The host plots are about to depend on exactly that. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…task period Counting pulses in a fixed 10 ms window leaves a +/-1 count ambiguity no matter how fast the shaft turns. At 64 apertures one count is 2*pi/64 rad, so the velocity quantum was 9.82 rad/s (~94 RPM) and the derivative multiplied that into +/-982 rad/s^2 of pure quantization noise on a perfectly steady shaft. Below ~94 RPM no pulse landed in some windows at all and the numCounts == 0 guard reported a hard zero, so slow rotation read as stopped and then jumped. The interval is now bounded by real pulse edges rather than by whenever the task happened to run. The ISR stamps each edge; the task measures from the pulse that ended the previous interval to the pulse that ended this one. Over such an interval the angle is exactly counts * (2*pi/apertures) -- no partial aperture at either end -- so the only remaining error is the timestamps' 1 us resolution. Simulated against known speed profiles, mean velocity error falls from 4.35 to 0.035 rad/s at 1000 RPM and peak acceleration noise from 982 to 8 rad/s^2; at 300 RPM the acceleration noise drops from 982 to 3.4. On a fast ramp the gain is smaller (about 2x) since real acceleration dominates there. While no pulse has arrived the shaft cannot have covered another aperture, so velocity is reported as that decaying upper bound, floored by the last known speed, instead of snapping to zero. Also fixed: the first sample was computed against prevTimestamp = 0 while TIM2 free-runs from boot, so it was divided by the entire uptime -- wrong by an unpredictable factor, and meaningless after a wrap. The task now adopts the first pulse as its reference and derives nothing from it. The reference is dropped when the encoder is disabled, since the ISR keeps counting and resuming against a stale edge would attribute an idle period's pulses to one interval. The arithmetic moved to encoder_math.c: pure, HAL-free and unit tested on the host against hand-computed physics, including the resolution claims above, the decay bound, degenerate sysconfig (zero apertures must not yield an infinity that flows into torque), and an interval spanning a counter wrap. OP_IN (PF9) sits on EXTI9_5, which CubeMX enables only because BTN_SELECT shares the line -- move that button and the encoder silently stops counting. The line is now claimed explicitly in a USER CODE block. Removed the unused num_overflows and timestamp statics, CalculateRPM, and the declared-but-never-defined ToggleOpticalEncoder. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…estamps The plots showed a rolling 30-second window on a host stopwatch. They now show the whole run: the x axis starts at 0 and extends to the newest sample, labelled in elapsed seconds, and time comes from each sample's hardware timestamp rather than from when the host happened to process it. Host-side scheduling jitter can no longer smear the trace, and the axis lines up with the device_ts column in the telemetry CSV. Stopping a session freezes the trace where it is -- the right edge is the newest sample, so it simply stops advancing. A later session is appended at its own timestamps, after however much time actually passed, so the buffers no longer clear on session start. Clear is now an explicit button, and the only thing that empties them. A silence longer than a second breaks the line instead of being spanned by a straight segment: a stopped session, or a dropped link, is a real hole in the record and drawing through it would invent readings nobody took. The threshold also clears a few decimation buckets, so once a run is long enough that a pixel column spans seconds, ordinary dense data is not mistaken for a gap. The x tick step is chosen per frame off the 1/2/5 sequence, since the same axis has to stay readable at five seconds and at an hour. Two supporting pieces: TimestampUnwrapper folds the counter's rollovers -- every 71.6 minutes now that a tick is 1 us -- into continuous seconds, so a long run plots as one timeline. It reads each step as the shorter way round the counter, which makes a post-rollover sample a small step forward and a straggler from just before one a small step back. The first cut instead accumulated whole laps, and a test of exactly that straggler case caught it placing such samples 71 minutes into the future. TimeSeriesBuffer now grows as it fills rather than allocating for the worst case, because "since the session started" needs far more depth than a trailing window: 4096 samples to start, doubling to ~87 minutes per channel at 100 Hz. Past the ceiling it drops the oldest and records that it did, since at that point the left edge is no longer the start of the run. Existing buffer tests that meant to exercise ring wraparound now pin the ceiling explicitly, so growth does not quietly turn them into no-ops. Verified in the app under Xvfb against two runs separated by a 40 s stop and straddling a counter rollover: the axis reads 0-110s, the gap is a real break in the line, and the rollover is invisible. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The device computed torque and power and streamed them, which meant the constants behind them were baked into every recording: getting the moment of inertia or the force-sensor lever arm wrong made past runs permanently wrong, and fixing the next one needed a rebuild and a reflash. With the inertia still unmeasured (0), that was the wrong place for those numbers to live. The device now streams only what it measures. The host derives torque = I*alpha + F*r, power = torque * omega, and the geared torque, from the raw force and encoder streams. Correcting a constant recomputes what follows, and the raw telemetry CSV keeps only measurements, so a run stays reproducible rather than frozen. Force and encoder samples come from independent tasks at different rates and never share a timestamp, so a reading is produced whenever either arrives, paired with the latest of the other -- the same sample-and-hold the firmware did. Nothing is emitted until both have been seen: the firmware started from a zeroed struct and published torque derived from a force of zero before the load cell had ever reported. The LCD shows force and RPM now, the two quantities the device still measures, so the dyno reads out usefully with no computer attached. Moment of inertia, lever arm and gear ratio become host-only, in a PC Constants section on the SysConfig page, stored in the app's database and never sent. Nothing in the firmware reads them any more, so they are gone from config.h and from the sysconfig catalog -- and gear ratio stops being a compile-time define the host had to parse out of a header. Removing two sysconfig parameters renumbers every id after them, because ids are positional. A database written before this holds rows under the old numbering, and reading them back by id would hand each parameter its neighbour's value -- a PID gain landing in a mechanical constant with nothing to show for it. The store now re-keys saved values by their stored name when it opens, and forgets names the catalog no longer has. That renumbering is also why the protocol version moves to 6: a v6 host pushing to v5 firmware would write gains into the wrong parameters, and the handshake refuses the link instead. Verified: firmware builds and 47/47 firmware tests pass; 174/174 host tests including the derivation, the id migration and PC constant persistence; schema drift checks clean; PC Constants section rendered under Xvfb. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Export CSV sits next to Add graph and writes everything currently recorded, through a save dialog. The format is long and sparse: one row per instant, carrying only the fields actually measured then, with X everywhere else. That shape is forced by the device rather than chosen -- force, encoder and BPM samples come from independent tasks at different rates, stamped from the same counter but never at the same moment, so almost every row holds one source and nothing else. Fields that were recorded from a single message do share a row, because they share a timestamp exactly: velocity with acceleration, and torque with geared torque and power. X rather than blank, as asked. Worth knowing when opening the file: spreadsheets read a column containing X as text, so charting one means replacing the marker first. The time column is elapsed seconds from the run's first sample -- the same number the plots put on their x axis, and the device's own clock rather than the host's. A stop/start therefore shows up as a jump in elapsed time, since a row exists only where something was measured. Values are written at full float precision: the file is the record, and re-deriving torque from it should not accumulate error the plots never had. Verified beyond the unit tests by replaying a realistic interleaved stream -- encoder at 100 Hz, force offset and faster, BPM slower, torque and power derived exactly as the app does -- and checking the arithmetic by hand: with I=0.02, r=0.25 and a gear ratio of 3, a force of 40 N against a held 60 rad/s gives 0.02*2 + 40*0.25 = 10.04 N.m, 30.12 geared, 602.4 W. The first encoder row shows X for torque, since force had not arrived yet and nothing is derived until both have. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The time column is now device_ts, the counter value the device stamped the sample with, rather than seconds since the run began. It is the same value the raw telemetry log records under that name, so an export and a logs/telemetry-*.csv from the same run can be matched row for row. The plot buffers hold elapsed seconds, since that is what the axis draws, so each is converted back. That is lossless only because a tick offset is a whole number and a double carries whole numbers exactly at these magnitudes -- pinned down by a test that round-trips 5000 samples per start point, including across a rollover, since a silent off-by-one-tick would misalign the export against the log it exists to be joined with. The origin is now kept in ticks rather than seconds so there is an exact integer to add back to. The exporter no longer decides how an instant is rendered; the caller passes the column name and a formatter. That keeps the row-shape tests about row shape, and leaves the choice of time base with the thing that knows the device. One consequence, verified rather than assumed: device_ts is the raw 32-bit value, so on a run crossing the counter's ~71.6-minute rollover the column wraps and reads backwards (4294967000 then 2704), even though the rows stay in true chronological order. That is the price of matching the log exactly; an unwrapped 64-bit tick count would stay monotonic but no longer line up with device_ts there. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Torque, geared torque and power were derived whenever *either* a force or an encoder sample arrived, each stamped with whichever measurement triggered it. Those come from two independent device tasks whose timestamps are not mutually ordered -- a few ms of inter-task skew is normal -- so a single plot buffer was being fed times that could step backwards. TimeSeriesBuffer documents non-decreasing times as a precondition, and three consumers rely on it: CopyWindow binary-searches on it, Envelope.Decimate's "output fits in 2 * bucketCount" bound assumes it, and the CSV exporter's k-way merge assumes each channel is sorted. The visible symptom was the export: torque and power rows came out in whatever order the two tasks interleaved. Derive on force alone, against the held omega/alpha of the most recent encoder sample. Force is the right clock to keep -- it runs 10x faster than the encoder (1 ms against 10 ms) and carries the term that actually moves during a pull -- and nothing is lost by not emitting on the encoder, because omega and alpha are held values either way. A fresh one simply applies from the next force sample, at most a millisecond later. Deriving on the encoder instead would have cost 10x the resolution. Also assert the invariant in TimeSeriesBuffer.Add. It was documented but unenforced, and a violation is silent everywhere -- the plot just draws a line that doubles back. Debug-only, so release builds are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
TimeSeriesPlot copied a series' whole window twice on every render: once in EffectiveRange to fit the y-axis, then again in the draw loop. The window is the entire run -- t0 is always 0 -- so a copy is the whole recording, up to half a million samples per channel, and the axis series was copied a third time. At depth that was the bulk of a frame's work. Split the range calculation into RangeFor(series, count), which reads the scratch arrays CopyWindow already filled instead of refilling them, so the per-frame copy count drops from 2N+1 to N+1. The axis series is still copied separately because the grid has to be drawn beneath the lines and each series overwrites the scratch. FixedRange is pulled out so a non-autoscaled series' range is still answered from its settings without touching the data -- the original did that, and folding it into RangeFor would have quietly started copying the axis series in the one case that never needed it. Draw order and empty-state behaviour are unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A resync tells the host that bytes were lost, but not which side lost them -- the firmware could have failed to send them, or the host could have failed to receive them. That ambiguity is what has made the 16-byte head loss hard to pin down. Every CDC transfer now ends with a usb_tx_batch_trailer carrying the sequence number and byte count of the batch the device handed its USB driver. The host weighs the bytes that actually arrived between two trailers -- decoded and skipped alike -- against what was declared, and reports the difference: a shortfall on a contiguous sequence means the device framed and submitted bytes that never landed, a sequence gap means a whole transfer vanished, and a resync with neither means the device framed the record wrong in the first place. The trailer is consumed by StreamParser as framing, never published as telemetry. It is written last, deliberately: the loss being instrumented eats the *leading* bytes of a transfer, which is exactly where a marker would be destroyed by the thing it is meant to measure. IsBufferFull reserves room for it at all times so TransmitBatch cannot fail to stamp a batch. Two fixes to the batch accounting itself: _batchSeq advanced on any result other than USBD_BUSY, including USBD_FAIL -- which Run() classifies, three lines later, as a batch that never left. The device burned a sequence number on a transfer it knew it had not sent, and the host duly reported "1 transfer never arrived, the device sent it and the host never saw it". That is precisely the misattribution this record exists to rule out, fired on the one case where the firmware already knows the answer. Only USBD_OK commits now; anything else rewinds the trailer and leaves the sequence alone. StallIfIsBufferFull also dropped a batch silently on USBD_FAIL, while the identical give-up path below it counted one. Flush-path drops were the one kind that never reached the event log -- silent precisely when the link is unhealthy enough to reach that function at all. Verified by the ARM build and the host suites (194 C#, 47 firmware). USBController.cpp is not compiled by fw_tests -- it needs HAL and FreeRTOS -- so the two firmware fixes are covered by compilation only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ostic)
The v7 trailer said which side of the link loses the bytes; it did not say
which layer of *this* side. Its wording ("lost below the firmware") reads as
an accusation of the firmware, but two facts rule the device out entirely:
- A 52-byte batch is one USB packet. The link is PCD_SPEED_FULL with
dma_enable off and a 64-byte max packet size, and bulk packets are
hardware CRC'd and retried -- a host cannot receive the last 36 bytes of
a 52-byte packet. The loss is not on the wire.
- TransmitBatch always submits from offset 0, so any device-side shortfall
truncates the tail. What the host loses is the head.
So the packet arrived whole and something discarded its first 16 bytes above
the wire and below the parser. Reading the host path found nothing that does
that, which is exactly why this records instead of guessing: capture what
SerialStream.ReadAsync returns, chunk boundaries and all, and replay it
through a fresh StreamParser offline.
The verdict is in whether faults reproduce. Replay is deterministic over
bytes already on disk, so a shortfall there is a shortfall in what the host
was given -- nothing above the driver could have caused it. A capture that
replays clean while the live run faulted says the opposite, and puts the bug
in our own live path.
Off unless DYNO_RAW_CAPTURE names a path, and the writing is done on its own
thread behind an unbounded channel: a slow reader is one of the suspects, and
an instrument blocking the read loop on file I/O could manufacture the very
stall it exists to observe.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two traps, both hit on the first real attempt at using it. The app is a WinExe, so on Windows it is a GUI-subsystem process with no console attached: run from a shell, everything --replay wrote to stdout was discarded and the command looked like it silently did nothing. Running it as `dotnet Dyno.App.dll` does get a console, but a diagnostic that only works when invoked one particular way is a diagnostic that gets reported as broken. The report is now also written to <capture>.report.txt, which survives either way. And FromEnvironment swallowed every failure to open the capture file, so a path whose directory did not exist yet produced no capture, no error, and a run that looked recorded until the fault had already been reproduced and lost. It now creates the directory, and hands back the reason when it still cannot open the file, so the event log can say so. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The first real capture answered the question it was built for -- one short batch in 1.1 MB, reproduced from bytes on disk -- and immediately raised the next one, which the report could not answer: the fault named chunk #32949 (39 B) but showed none of its neighbourhood, so there was no way to see whether the 16-byte gap fell between two deliveries or inside one. Those are different bugs. A gap on a boundary is a driver that skipped bytes between reads; a gap inside a single delivered buffer would mean one read returned internally discontinuous data, which is stranger and lives somewhere else. Each fault now carries the last six deliveries, printed one line of hex per chunk so the joins are the only thing in question. Faults run to single digits across a million bytes, so keeping the run-up costs nothing -- and not keeping it costs a whole capture-and-reproduce cycle on hardware to answer a question the existing recording already contains. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three graphs split the page three ways before anything is on screen, so the first thing the page shows is three short strips of nothing. One opens at full height, and Add builds up from there for whoever wants more. Angular velocity is the one kept: it is the only default that is measured rather than derived, so it is the one that shows something on a board with no mechanical constants calibrated yet. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Captured deliveries show every batch arriving as exactly two reads -- one of a single byte, then the remainder. Strictly alternating, ~2 chunks per batch across 28883 chunks. That is a read left pending being completed the instant the driver copies the packet's first byte, with the rest already buffered by the time the next read is issued: every packet is handled mid-copy. That is also the window the 16-byte head losses appear in. The context around the fault puts the gap between two deliveries, not inside one: the previous batch ended clean on its trailer, and the next 1-byte read carried payload byte 2 (6D) where it should have carried SOF byte 0 (5A). The driver's read pointer jumped 16 bytes between deliveries and the 1-byte cadence resumed. So pause after each read. Nothing is pending while a packet lands, the driver fills undisturbed, and the next read takes whole batches. 5 ms costs a few milliseconds on command acks against a timeout in seconds, and a few KB against the 1 MB the port is opened with. This is a hypothesis with a mechanism, not a proven fix -- I cannot derive why the jump is exactly 16 bytes. Hence ReadSettleDelay is a property and DYNO_READ_SETTLE_MS overrides it: run a session each way on one board and compare the short-batch counts. Zero restores the old behaviour exactly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The prediction held and the hypothesis still failed. Pausing between reads changed the delivery shape exactly as expected -- the 1-byte reads are gone, chunks went from ~28 bytes to ~132, and every chunk now starts and ends on a batch boundary -- and the 16-byte head losses continued unchanged: 2 faults in 949 KB, against 2 in 1.93 MB while racing. Same fingerprint, same 16 bytes, same position. So reads racing the driver's buffer fill is ruled out as the cause. The delay is kept, but for the 4.7x drop in reads it buys, not as a fix; the doc comment said otherwise and would have misled the next reader into thinking this was settled. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
OTG_FS_IRQn sat at NVIC priority 5, which is exactly configLIBRARY_MAX_SYSCALL_INTERRUPT_PRIORITY, so BASEPRI masked it and every taskENTER_CRITICAL() anywhere in the firmware delayed it. That is not survivable on this interrupt. With dma_enable = DISABLE the core arms the IN endpoint (EPENA|CNAK) before any data is in the TX FIFO and only fills it afterwards, from this ISR, in PCD_WriteEmptyTxFifo. Between those two moments the host may send an IN token, and the core answers it with whatever the FIFO holds -- splitting one batch into a short packet plus the remainder. Moved to priority 4, above the syscall threshold. Legal because nothing on the ISR's path calls a FreeRTOS API: CDC_Receive_FS pushes into the lock-free SPSC ring in usb_rx_ring.c, which never relied on interrupt masking. The override goes in the USER CODE block rather than on the generated line, so a CubeMX regen keeps it. Three latent bugs found alongside it: - EP2 IN (CDC_CMD_EP) had no TX FIFO at all. HAL_PCDEx_SetTxFiFo only writes DIEPTXFn when called for n, so DIEPTXF2 kept a reset value addressing past the end of the 320-word RAM. Dormant only because ST's CDC class never transmits on the command endpoint. The 16 words come from EP0, which keeps 3x its max packet; the EP1 sensor stream is untouched. - CircularBufferReader::GetElement(size_t) had taskEXIT_CRITICAL() after its return, i.e. unreachable. One call would leak a uxCriticalNesting level and leave BASEPRI raised for the rest of the run. Never fired only because nothing calls that overload. - configCHECK_FOR_STACK_OVERFLOW was unset, so the hook compiled out. Defining it was not enough: vApplicationStackOverflowHook lacked extern "C", compiled to a mangled symbol, and the linker silently took the __WEAK dummy from cmsis_os2.c. Set in the .ioc too, since CubeMX owns that key. Builds clean; 47/47 host tests pass. Not yet verified on hardware. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both were experiments that have served their purpose and were left in. The 5 ms settle delay was added to test whether the read loop was racing the serial driver's buffer fill. It changed the delivery shape exactly as predicted and the losses continued at the same rate, so that theory was ruled out; it was kept only for a side benefit, ~4.7x fewer reads. That is not worth what it costs. Every command ack pays two of them, which is why idle ping RTTs sat at a flat ~10 ms against ~1 ms without it -- and the RTT climb under load is one of the signals used to spot the stream fault in the first place, so a 10 ms floor was muddying the very measurement it sat next to. The 4.7s heartbeat interval was chosen coprime to the device's 1s task-monitor cycle, to tell whether ack delivery was quantized to the device's next transmit. Its own comment said to restore the 5s default once measured. The test covering the settle delay becomes a plain Fact: the setting is gone but the invariant it was protecting -- a burst of sends arrives complete, in order, exactly once -- still holds and is still worth asserting. RawCapture is deliberately left in place; the head-loss fix it exists to diagnose is not yet confirmed on hardware. 204/204 host tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Flashed and tested: the 16-byte head loss reproduced 2.3 s into the first session, batch #276, 68 of 84 B. The change was genuinely on the chip -- the Release .elf disassembled to movs r1,#4 with r0=101 after HAL_NVIC_EnableIRQ -- so interrupt latency from critical sections is not the mechanism. Reverted rather than kept on general principle. Priority 4 silently requires that nothing on the USB ISR's path ever call a FreeRTOS API. That holds today, but it is vendor code, and a future violation would hang in configASSERT's for(;;) with no obvious cause. It also made OTG_FS the only interrupt in the project not at 5. Neither is worth carrying for a theory that bought nothing measurable, least of all while still hunting the bug, where every unexplained change is a confound in the next experiment. The reasoning error is recorded in the comment: the loss is always exactly 16 bytes, across batch sizes of 52, 56 and 84 and across one- and two-packet transfers. A race splits at a different offset every time. This is structural, and the next theory has to explain the constant. The other three changes from c9437c5 stand -- they were independent bugs, not consequences of that theory. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every theory about the 16-byte head loss so far has been argued from host-side evidence, and two of them were wrong. This measures the one thing never actually measured: whether the device splits the batch at all. The OTG core has its own TX FIFO underrun flag, DIEPINT.TXFIFOUDRN -- exactly the "an IN token arrived and I did not have the whole packet to give" condition that would transmit a batch short. ST's HAL disables it (DIEPMSK &= ~TXFURM) and USB_ReadDevInEPInterrupt masks its DIEPINT read by DIEPMSK, so the driver never reads it and never clears it. The bit just latches, which is what makes it readable without touching vendor code. usb_otg_stats samples and clears it at OTG ISR entry, before HAL_PCD_IRQHandler -- the HAL clears ITTXFE itself, so a later sample would miss that one. Only TXFIFOUDRN and ITTXFE are touched, both of which the HAL takes no action on; XFRC is left strictly alone since the driver's transfer state machine runs off it. USBController reports movement as WARNING_USB_OTG_TX_FIFO_UNDERRUN, rate limited to one per second like the existing drop report, so it lands in the host event log beside the "dropped 16 bytes to resync" line it would explain. No protocol bump: this adds an enum value, not a wire-format change. Reading it is binary. Underruns tracking the resync warnings means the device really does split the batch and the firmware is in scope. Underruns staying at zero across a fault means the core transmitted everything it was handed, and no further firmware work can help. Firmware and host build clean; 47/47 firmware and 204/204 host tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This reverts commit 2a1ee3e. The probe could not have answered the question it was built for. A bulk IN endpoint does not underrun: if an IN token arrives and the TX FIFO does not hold a complete packet, the core NAKs and the host retries rather than transmitting a partial packet. That is what makes bulk reliable. DIEPINT.TXFIFOUDRN is a periodic-endpoint / threshold-mode concept and is effectively dead silicon on EP1 IN, so its silence carried no information. It also shipped without a validity check -- ITTXFE was the counter to report, since it should climb constantly on a polled bulk IN endpoint, and a zero there would have distinguished "no underruns" from "dead probe". Removed rather than kept, on the same reasoning as the interrupt priority revert: a run on the build carrying it produced a fault mode never seen before -- a seq gap, one whole transfer lost, where every prior event had contiguous seq and a 16-byte shortfall. No mechanism connects a masked-register read to a lost transfer, but an instrument that measures nothing has no claim to stay in while it sits next to an unexplained change in symptoms. What the exercise did establish is worth more than the probe: if the device cannot emit a partial bulk packet, it cannot have split the batch, so the earlier conclusion that the firmware was back in scope is wrong and the loss is on the host side of the wire after all. Firmware FLASH size returns byte-identical to the pre-probe build. 47/47 firmware and 204/204 host tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every fix here came out of one sequence: flashing while connected, and the several ways the link failed to recover from it. Link teardown could not be escaped. Disconnect awaited an unbounded client.Dispose, and closing a tty whose device node is gone blocks indefinitely -- with IsConnected cleared only after that await, and CanConnect gated on it, the window sat on "Disconnecting..." with Connect greyed out and no way back but restarting the app. Teardown is now bounded and an overrunning close abandoned; the loops are already cancelled, so what leaks is a thread blocked on a device that no longer exists. Reconnecting then crashed. The board reboots when the link drops -- its error timestamps read 219 ms of TIM2 on every connect -- so TIM2 restarts while the plots kept their clock and time origin. New samples arrived behind the origin, were floored at 0 by Elapsed's skew clamp, and landed in buffers whose latest time was 18 s. Debug caught it; Release would have silently drawn a trace doubling back, with CopyWindow's binary search and the CSV export reading a non-monotonic buffer. A link boundary now resets the plots, which a session boundary still deliberately does not: two runs on one counter are separated by real elapsed time, but no origin reconciles two boots. And it needed doing by hand, because a rebooted board comes back on whatever ttyACM number is free. PortAlias names it by udev's persistent symlinks (by-id first, which carries the serial number and so tells two boards apart; this project's /dev/dyno as fallback). On losing the heartbeat the link asks whether its node still exists -- gone means re-enumerated, and it follows the alias to wherever the board reappears. Merely unresponsive is left alone: the client keeps pinging and recovers on its own. Flashing no longer does this to itself. The Firmware page had no idea the link existed and programmed the board underneath it; it now releases the link first, restores it after (in a finally, so a failed or cancelled flash still gets it back), and stays busy throughout, since buttons going live mid-poll let a second flash start just as the watcher reconnected. Two things the board and the app disagreed about: Reset with the brake held lit the brake LED, showing an engaged brake while the FSM came up idle. The level alone never started a session -- EXTI fires on edges -- but an edge latched during boot sat in the buffer and the FSM read it from index 0. It now starts where the ISRs have got to, and a brake already held at construction starts disarmed: the release arms it, so running the BPM takes a deliberate press made since the reset. The app assumed the session state rather than checking it. The firmware states it after every ack precisely so a host arriving at a steady board learns it without an edge, but Start() encoded "unknown" as false, so an idle board's first announcement de-duplicated into silence. It is now tri-state: the first statement always raises, idle included, and losing the link returns to unknown rather than claiming a session stopped that may still be running unseen. Also stops a stray empty CSV per failed connect: the log was opened before the port, which the retrying watcher would have turned into dozens. Firmware builds clean in the Docker toolchain; 215 tests pass. Untested on hardware -- the firmware needs flashing, and the reconnect timings (2 min window, 1 s settle) are judgement calls worth tuning against a real flash. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DEBUG_USB_CONTROLLER_MOCK_MESSAGES was a compile-time gate: exercising the link with no sensors attached meant a rebuild and a reflash. It is now SYSCFG_USB_MOCK_MESSAGES, an ordinary runtime parameter, default disabled. Deliberately with no config.h #define behind it. Every other parameter's macro is its boot default, which is what a compile-time setting for it would mean here -- but there is no build this one has to agree with, and a macro would only suggest that rebuilding with it changed something. So both generators now take a `default:` from the schema instead of requiring a #define, and the firmware table emits the literal. One or the other is required; neither is still an error. Because it can now be switched on while someone is watching the plots, the Plots page says so: an amber banner reading FAKE DATA whenever the setting is non-zero, driven from the saved value (which is what the host pushes after every handshake, so it is also what any connected board is running, and it is right before one is even connected). Nothing about a counter's trace would otherwise give it away, and an export taken during one is fabricated too. usbcontroller_main loses its compile-time either/or and always calls Run(); the loop's streaming step branches on the parameter and calls AppendMockFrames in place of the sensor reads. Commands, health, faults, framing and flush are untouched, so only the numbers are invented. inSession is forced true while mock is on -- kept at the session flag rather than at the streaming step, so the session announcement goes out too and the host is told what it is seeing. The canned error/warning pair goes out at most once a second rather than once per loop. They exist to exercise the error framing, and at the task's rate one per loop put hundreds of fabricated lines a second into the host's event log, burying every real one: a diagnostic mode whose diagnostics cannot be read. WaitForHandshake() was the old mock path's only caller and is removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…stants Closing the window while connected hung the app. Shutdown() blocks the UI thread on TearDownAsync, which awaited without ConfigureAwait(false) and so posted its continuation to the very dispatcher that was waiting for it. That deadlocks however healthy the port is, which is why it only ever reproduced while connected. Regression from 095aecb, which replaced a plain Task.Run(Dispose).Wait(timeout) with the async helper. TearDownAsync now awaits context-free, and says in its remarks that it must stay that way. The Plots page could take the process down with it. Elapsed() floored a negative time at 0 and handed that to buffers that require non-decreasing times, so a device whose counter restarted under a live link -- a board that reset, or a stream that switched to the mock counter -- asserted in a debug build ("got 0 after 12.529881") and silently drew a run doubling back on itself in a release one. A sample more than a second behind the newest one recorded is now read as a new time base: the run ends and a fresh one starts at 0, with a line in the event log saying why. Ordinary lag between sensor tasks (the BPM channel's newest point is legitimately older than the encoder's) is far inside that and still plots where it falls. PC constants behaved unlike every other row on the page: - No default anywhere. The value existed on the view model and was shown nowhere, so a constant edited by mistake had nothing to go back to. Each row now carries the same hint a runtime parameter does ("default 1 · 0.000001 to 1000") and a Reset that stages it. - Apply left the page claiming the change was still pending. PcConstantViewModel was the only MarkSaved of the three that did not recount, and re-formatting the text usually raises nothing, so the count stuck at its pre-Apply value forever -- and, since a PC-constant save reported no status at all, an Apply that had in fact written to SQLite looked like one that had done nothing. It recounts now, and the section has its own status line: these never reach a device, so "applied to the device" would be the wrong reassurance and its absence would read as a failure. - A rejected value greyed Apply out with only a small red line beside the description to say why. The box now takes the red border the runtime rows use. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both indices wrapped at `size`, which made a full buffer and an empty one the same state: write exactly `size` elements and the writer lands back on an untouched reader, the two compare equal, and every reader's "is there anything here" test -- which is that comparison -- says empty. A whole buffer of unread data was dropped without a trace. That is not a hypothetical shape for the task-error buffer. It is the one buffer meant to hold a backlog: errors are kept until a host has handshaked, deliberately, so that a fault at boot reaches whoever connects later. Fifty faults before anyone attaches and the log comes back empty -- and the boot faults are exactly the ones worth having. The indices now count writes and never wrap, with `% size` applied only where an element is addressed. `size` writes leave the writer `size` ahead, which is unambiguous at any capacity. The counters do wrap at 2^32; the arithmetic is unsigned subtraction, so distances stay right across it. Counting also makes being lapped a distance the reader can test, and it has to test it. A reader that fell behind used to sit on stale slots but at least advanced one per read; with counting and no clamp it would see `writer - reader` elements available and hand a drain loop the same slots round and round, as many times over as the writer got ahead. So a lapped reader rejoins at the oldest element that still exists and adds what it skipped to TakeDroppedCount(). Losing the oldest when a producer outruns a consumer is what a ring is for and is not a fault; what is new is that the loss is ordered and countable rather than silent. Nothing reports the count yet. Index values handed between the two -- SkipBufferedSensorData giving a reader the writer's position to drop a backlog -- are write counts now, and are stored as given rather than reduced. TASK_ERROR_CIRCULAR_BUFFER_SIZE 50 -> 100. It had the least margin of any buffer despite being the smallest, since it is the only one not drained every pass of the USB task. The test that pinned the old behaviour asserted a buffer filled to capacity reads as empty; it now asserts the opposite, and a new one laps the reader by half a buffer and checks it gets the survivors once each, oldest first, with the loss counted. 48 firmware tests pass and the Docker build is clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…g it The circular buffers now count what a lapped reader lost, but nothing did anything with the count. The USB task takes each reader's tally every pass and frames a warning per buffer, at most one a second per stream: four new schema codes, one each for the optical encoder, force sensor, BPM and task-error buffers. Per buffer rather than one "something overflowed", because which one it is is the diagnosis -- a single stream means that producer outran the drain, all four at once means the USB task itself stalled. Collecting and reporting are separate steps for a reason: TakeDroppedCount clears as it reads, so a lap that happened while a report was not yet due would otherwise be forgotten. They are warnings, matching WARNING_USB_TX_BATCH_DROPPED -- data is lost but the link and the session carry on. The tallies clear on host detach, since a backlog from a previous host says nothing about this one. Reporting them made a longstanding gap plain: the event log rendered faults as `TASK #0`, so the new codes would have arrived as USB_CONTROLLER #1 through #4 and told nobody which queue. ErrorDecoder.Name now resolves any packed code to its schema name through the producing task's generated enum -- the number alone means nothing, being task-local -- so a line reads FORCE_SENSOR_ADS1115 FORCE_SENSOR_ADS1115_INIT_FAILURE rather than #0. The name is kept as the source identifier rather than turned into prose so a line in the log greps straight back to the firmware that emitted it. A code this build has no name for still falls back to the number, so an app talking to newer firmware degrades instead of breaking. The four overflow codes get a written-out line above that, since "this run has a gap" is the part worth saying in words. A theory test drives every value of every *_error_ids enum in the generated contract through the decoder, off reflection rather than a list kept beside it, so a fault added to the YAML that nobody wires up fails a test instead of quietly printing a number. 234 host tests, 48 firmware tests, Docker build clean. Two things this does not cover. The count of lost samples cannot ride along: task_error_data is {timestamp, error_code} and stays that way, so the message says which buffer and when, not how many. And the osMessageQueues are not included -- a full queue is rejected at the writer, which is the pushing task's to report, not something the USB task can observe. Untested on hardware: provoking a real overflow means stalling the USB task or overdriving a producer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…reads from A fault reached the event log as `TASK #3`. The number is task-local, so it means nothing on its own, and what a reader actually wants -- what broke, and what it costs the run in front of them -- was prose hand-written in the app, for the four codes someone had got round to. Two mechanisms, neither of them where the fault is defined: a task-to-enum map for names, a string switch for the buffer-overflow warnings, and silence for everything else. All of it moves into messages_public.yaml, beside the code it belongs to. Fault enums gain `task:`, naming the task_offset_t their numbers belong to, and every value gains a `description:` -- the sentence the app prints. Seventeen faults, each written from the firmware that raises it, so they say what actually follows: which tasks park themselves and need a reset, which back off and retry, and what is missing from the run either way. error_msg_generate.py renders that to ErrorCatalog.cs, one row per fault keyed on the packed 32-bit code, which is the only sound key -- the number is reused by every task that defines one. It is a separate script from generate.py on purpose. That one renders the wire contract, where every value is a C expression it has to evaluate and both sides must agree byte for byte; this renders documentation, one row per enum *value* across enums, answering to the UI. It imports the C-expression evaluator and the string-literal helper rather than copying them, so the two cannot come to disagree about what a value means. It refuses to generate on a missing or unknown `task:`, a missing `description:`, a name prefixed with neither ERROR_ nor WARNING_, or a prefix that disagrees with the warning bit in the value. Failing the build is the point: a blank row is the exact thing the catalog exists to prevent. The app side collapses to one case. ErrorDecoder is back to just Decode -- its reflection over the generated enums and its hand-written task-to-enum map are gone, and so is the buffer-overflow string switch. Where a fault name starts with its own task's name the repeat comes off, so what used to read FORCE_SENSOR_ADS1115 FORCE_SENSOR_ADS1115_INIT_FAILURE now reads FORCE_SENSOR_ADS1115 INIT_FAILURE, still whole for anyone grepping the firmware. An unknown code keeps its number and its task, which is everything genuinely known about it. scripts/generate.sh and generate.ps1 run both generators, forwarding the same arguments; each skips a --target it does not own, so --target error_catalog still reaches the one that has it. check.py loops over both and names the owning script when it fails, so CI covers the catalog with no workflow change -- worth having for this file in particular, since descriptions read like prose and the generated file is the tempting place to fix a typo. The new schema keys are ignored by the firmware generator; the C header is byte-identical and the Docker build is unchanged. ErrorCatalogTests holds the catalog against the generated enums in both directions -- every firmware fault has an entry whose low bits match, every entry names a real fault -- which is a real check rather than a tautology, the two files being rendered from one schema by two different generators. 238 host tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The bounds on the SysConfig page were round numbers nobody derived: every task delay ran to 60000, the load cell was configurable to 1e5 lbf, the encoder wheel to 100000 slots. They rejected nothing anyone would actually type and documented nothing about the parameter. Each integer parameter now carries the full range of the type its value logically is -- a millisecond delay is a uint16_t, so 0..65535; a retry count is a uint8_t, so 0..255 -- which is a fact about the value rather than an opinion about what a sensible one would be. `type:` stays uint32/float because that is the wire encoding; the narrower type lives in the min/max. Floats have no width to read a range off, so theirs is invented at +-1e9: out of the way, and finite so a typo still cannot arrive as inf. The duty cycles keep 0..1, where the fraction is the type -- SetDutyCycle multiplies it into a uint16_t compare register, so above 1 the brake goes soft rather than hard. The bottom of every integer range is now 0, osDelay included. A task that stops yielding is reachable on purpose: these bounds exist to stop a value that cannot be represented, not one that is a bad idea. IntegerRangesAreWholeTypeWidths fails on any integer bound that is not a type width, since the catalog is generated and nobody reads that diff. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both log tabs share one ListBox, so "don't scroll the events tab, its newest line is already at the top" did not leave it at the top -- it left it wherever the Console tab had last pinned the shared ScrollViewer, which is the bottom. Opening Errors/Events therefore landed on the oldest event in a list that grows upward. So there is no longer a tab that is followed and a tab that is not: both are pinned to their newest line, at whichever end that tab puts it. Scrolling away from that end still pauses the follow, which for events means scrolling down. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…he real The gearing multiplied torque and multiplied angular velocity, by the same ratio in the same direction. A 4:1 reduction therefore reported 4x the torque and 4x the speed at the output -- 16x the power actually measured, produced by a gearbox. An ideal gearbox trades speed for torque; it does not make either. Velocity is now divided by the ratio, so the geared pair carries the same power as the sensed pair. The two halves live together as GearTorque/GearVelocity, because the drift is what the split caused: torque was geared in the derivation while velocity was open-coded in two view models, with nothing to say they were one relationship. The new test asserts the invariant rather than the formula -- geared torque x geared omega == torque x omega -- which is what would have caught this. A ratio that is not finite and positive now reads as direct drive: PC constants are range-checked as they are typed but not as they are loaded, so a bad row would otherwise put an infinity on the readout. Recorded runs and exported CSVs have the wrong angular_velocity_geared_rad_s; every other column was always right. Switching the mock stream on or off also now throws away everything derived from the setting before it: the run on the Plots page, the held force/omega the derivation pairs across streams, the live readouts, and the telemetry CSV, which is rolled to a new file rather than cleared -- it is a record, and the honest form of "these rows are not comparable" is two files, not one file with a seam. It also avoids the mock stream's timestamps, which restart at zero, tripping the time-base guard and blaming the board for resetting. ClearTelemetry left torque, geared torque and power lit at their last values, so a disconnect blanked the measured readouts and left the derived ones looking current. They are cleared with the rest now. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…king The page spent its width on things a reader cannot act on: which header a setting is declared in, where the config directory is, where the settings database lives. The file badge was also what clipped the longest section name -- "Voltage Reference (should be 3V3)" -- since it took fixed width from a heading that had to share the row. Badge gone, heading wraps, and the line under an overridden setting says what the firmware has rather than which file says so. The status lines keep the failures, which are actionable, and drop the paths. ConfigCategoryViewModel.FileLabel went with the badge; the grouping still keys on the file, because two headers may use one section name. The PC Constants blurb stated the torque and power equations, which is a fact about this app's arithmetic rather than about the constants being edited. It now says what they are for. Same for the two descriptions that named terms of those equations. The Reload button is gone. It read as the opposite of Apply and was not: Apply saves what you typed, Reload re-read what the firmware source says, discarding staged edits as a side effect. But it covered something real -- the headers are read once at startup and nothing here writes them, so a pull or an edit elsewhere left the page describing a firmware that is no longer there. That is now done on the way into the page, as the Firmware page already does, and skipped while an edit is staged, since re-reading rebuilds the rows and would throw typed work away. The force sensor lever arm now defaults to 0.1 m. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Raised on every header load and after each compile-time Apply, and listened to by nobody. Its summary said it let readouts derived from a compile-time setting refresh without polling the page, naming the gear ratio -- which stopped being compile-time when torque and power moved to the host, and is now a PC constant read through PcConstant(). So the one caller it was written for had already gone, and what remained described a mechanism the app does not have. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
"Export CSV crashes the app" was the file dialog, not the CSV. The picker call sat outside the try that wrapped the write, and the handler is async void -- so an exception escaping it is returned to no caller. It reaches the synchronization context unobserved, and with no global handler installed that ends the process. Nothing in the picker call looks like it can throw, which is what made the placement look safe. On Linux it is not in-process at all: it is a DBus request to xdg-desktop-portal, which can be absent, refuse, or never answer, for reasons that have nothing to do with this app. So the one await most likely to fail on a user's machine was the one await not covered. A timeout now says which piece did not respond and that a portal backend is what to check, since "export failed -- The operation has timed out" sends nobody anywhere useful. The exception type joins the other messages for the same reason. A missing storage provider used to return in silence, leaving a button that did nothing; it says so now. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Settings were reproducible only by retyping them one row at a time, which is how a bench setup that worked stops being recoverable once someone changes a value and cannot say what it was. The file is keyed by name -- K_P, GEAR_RATIO, USB_TX_BUFFER_SIZE -- not by wire id or position, so it stays readable and survives a schema change that renumbers ids. The cost is that a name this build does not have cannot land on whatever parameter now sits at that number; it is reported and skipped instead. Compile-time values stay strings, because they are C tokens rather than quantities: 0.95f and 16 + 1 do not survive a trip through a JSON number. Reading is hand-rolled over JsonNode rather than bound to a DTO. The file is something a person may write by hand or paste together, and a binder throws on the first type mismatch and takes every other setting in the document with it. One bad entry now costs that entry. Only a document that is unusable whole -- not JSON, or a root that is not an object -- is rejected outright, since a half-applied import leaves the page holding a mix nobody chose. An import stages rather than applies. A runtime value goes to a connected board the moment it is applied, and the file may be one someone else wrote, so the user gets to look at what arrived, on a page that already marks every changed row, before any of it reaches hardware. Apply remains the only commit point. Every setting the page knows gets a value -- the file's if usable, otherwise its own default -- so an import describes a whole configuration and not a patch. Out-of-range values are treated as absent rather than staged: the firmware would refuse them with MALFORMED, and staging one leaves the page un-appliable until the user finds the row by hand. What the file left out is one line per kind, not per setting. A partial file is the normal case, and eighty lines would bury both the real warnings and whatever the log already held. Export writes the values in force, not the ones staged in the boxes, which is what makes the file a description of a configuration rather than of a half-finished edit. The view holds the two dialogs because they need the window; both keep every await inside the try, for the reason the CSV export just demonstrated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The read loop knew the port had failed and said nothing, so an unplug was only noticed when the heartbeat had missed twice — five to ten seconds of a dead board still presented as connected. It now reports the loss on both of its terminal paths, the failed read and the end of stream, and stays quiet when cancellation was already requested: that case is our own Disconnect closing the port to unblock the read, not the board vanishing. Which of the two happened has to travel with the event, hence LinkLoss. Reporting instantly beats udev to the news often enough that the node is still listed when the view model looks, so asking "is the port still there?" would answer "yes" about a board that is already gone. The reason now says so directly, and PortHasVanished stays only as the second way to reach the same conclusion. Also folds in this file's share of the message shortening: it sits in the same hunk as the OnConnectionLost change and would not split out cleanly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
"No Session" was being cut off, and not only on small screens: the pill shared a fixed-width column with the port controls, so a long status string pushed the row wider than the window at the default size too. The pill now sits in the star-sized column and trims, with the full text on a tooltip. The trimming needs the inner Grid: the horizontal StackPanel it replaces measures its children with unbounded width along its own axis, so the TextBlock never learned it was short of room and TextTrimming would never have fired -- the text would run under the pill's edge and stop, with no ellipsis to show it had been cut. The port ComboBox gives up 60px of MinWidth so the pill still has room to say something at 780px wide. The task row had the same unbounded-measure problem and gets the same fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The event log had grown into advice -- what the cause probably was, what to check, what is safe to do next -- which buries the one thing the line exists to report. Each message now states the problem and stops. "staged, not saved" stays on the config import: that is the state the page is now in, not counsel about it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Export failed with "Portal operation not allowed: Unable to open /proc/<pid>/root". xdg-desktop-portal resolves who is calling before it will act, and when that resolution fails it refuses the request outright rather than falling back to treating the caller as unsandboxed -- so this is a failure to identify us, not a permission we were denied, and there is nothing to grant in answer to it. The machine already allows everything the check needs: /proc carries no hidepid, ptrace_scope is 0, the portal runs as the same uid, and our SELinux label is unconfined. UseDBusFilePicker = false takes the portal out of the path; Avalonia falls back to its GTK or managed dialog. Verified by watching the session bus: with the portal enabled a save picker emits one FileChooser message, and with this set it emits none. The option is X11-only and inert elsewhere, so Windows keeps its native IFileDialog and never had this failure mode. The cost is a dialog that is not GNOME's, so no GNOME bookmarks or recent files. The try/catch around the pickers still earns its keep regardless. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
778c4da gave every integer parameter the full range of the type its value logically is, so the osDelays went from [1, 60000] to [0, 65535]. It updated the C# catalog test to match but not the firmware one, which still asserted that 0 and 60001 were rejected. Both now pass the store, so the test failed. The rejection test loses its below-min case rather than gaining a new value: with every integer range starting at 0, an unsigned write cannot fall under one, and only the top can be exceeded. U32RangeBoundariesAreInclusive was stale without failing -- it probed 1 and 60000, which sit inside the new range, so it passed while naming boundaries that had moved. It probes the real ones now. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The module READMEs were frozen at f42989f while the code moved through protocol v5, v6 and v7 and a rewritten encoder. What was left did not merely omit things, it contradicted the source -- the worst kind of documentation, since a reader has no way to tell which half is lying. The flat wrong claims: usb_message_flow.puml said outbound records are unframed on the grounds that USB-CDC is reliable, which v5 disproved and framed. It also predates the v7 batch trailer, the per-buffer overflow warnings, and the sysconfig command it never covered at all. Dyno.Core/README documented the inbound stream as unframed too, and explained IsPlausible at length -- a function v5 deleted. The parser it describes scans to a SOF and checks a CRC. SessionController claimed to compute torque and power, with the formulas. v6 moved both to the host; FiniteStateMachine.cpp already says so in a comment. OpticalSensor described counting pulses per task period, including two functions that no longer exist. The task measures between pulse edges now. MessagePassing presented the generated headers as hand-written, with no framing, no version, and no mention of the schema they come from. Config had no SysConfig store in it at all, and still pointed at a mock-message flag that is now a runtime parameter. TimeKeeping said wrap is not handled. Both consumers handle it. Also ADS1115_SAMPLE_SPEED -> ADS1115_RATE in two sensor docs, and the Core module table's "torque/power math". Each claim here was checked against the source rather than against the commit that changed it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Errors/Events tab inserts each new line at index 0, which shifts the selected item's index; ListBox.AutoScrollToSelectedItem (on by default) then scrolls that selection back into view, yanking a user reading the backlog. The code-behind's _pinned follow governs only its own scrolls and never sees this internal one, so the earlier follow-newest fix left it standing. A console tab appends after the selection and was unaffected, which is why it only showed on the events side. Hand all scrolling to the _pinned machinery by turning the built-in auto-scroll off. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
WriteLine emitted Environment.NewLine, so the exported file — and the test output — carried \r\n on Windows and \n on Linux. The row-shape tests split on \n and saw a stray \r on every line, failing only on the Windows CI runner. Fix the format at the source: the file is the record, so its bytes should not depend on where the app happens to run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Author
|
fix this Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead. (https://xunit.net/xunit.analyzers/rules/xUnit2029)build / build & test (ubuntu-latest)View detailsCollapse annotationCheck warning on line R65Check warning: Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead. (https://xunit.net/xunit.analyzers/rules/xUnit2029)build / build & test (ubuntu-latest)View detailsCollapse annotationCheck warning on line R65Check warning: Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead. (https://xunit.net/xunit.analyzers/rules/xUnit2029)build / build & test (ubuntu-latest)View detailsCollapse annotationCheck warning on line R65Check warning: Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead. (https://xunit.net/xunit.analyzers/rules/xUnit2029)build / build & test (ubuntu-latest)View detailsCollapse annotationCheck warning on line R65Check warning: Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead. (https://xunit.net/xunit.analyzers/rules/xUnit2029)build / build & test (windows-latest)View detailsCollapse annotationCheck warning on line R65Check warning: Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead. (https://xunit.net/xunit.analyzers/rules/xUnit2029)build / build & test (windows-latest)View detailsCollapse annotationCheck warning on line R65Check warning: Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead. (https://xunit.net/xunit.analyzers/rules/xUnit2029)build / build & test (windows-latest)View detailsCollapse annotationCheck warning on line R65Check warning: Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead. (https://xunit.net/xunit.analyzers/rules/xUnit2029)build / build & test (windows-latest)View details Comment view Assert.Empty(ErrorCatalog.All.GroupBy(f => f.Code).Where(g => g.Count() > 1));Collapse annotationCheck warning on line R74Check warning: Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead. (https://xunit.net/xunit.analyzers/rules/xUnit2029)build / build & test (ubuntu-latest)View detailsCollapse annotationCheck warning on line R74Check warning: Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead. (https://xunit.net/xunit.analyzers/rules/xUnit2029)build / build & test (ubuntu-latest)View detailsCollapse annotationCheck warning on line R74Check warning: Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead. (https://xunit.net/xunit.analyzers/rules/xUnit2029)build / build & test (ubuntu-latest)View detailsCollapse annotationCheck warning on line R74Check warning: Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead. (https://xunit.net/xunit.analyzers/rules/xUnit2029)build / build & test (ubuntu-latest)View detailsCollapse annotationCheck warning on line R74Check warning: Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead. (https://xunit.net/xunit.analyzers/rules/xUnit2029)build / build & test (windows-latest)View detailsCollapse annotationCheck warning on line R74Check warning: Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead. (https://xunit.net/xunit.analyzers/rules/xUnit2029)build / build & test (windows-latest)View detailsCollapse annotationCheck warning on line R74Check warning: Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead. (https://xunit.net/xunit.analyzers/rules/xUnit2029)build / build & test (windows-latest)View detailsCollapse annotationCheck warning on line R74Check warning: Do not use Assert.Empty to check if a value does not exist in a collection. Use Assert.DoesNotContain instead. (https://xunit.net/xunit.analyzers/rules/xUnit2029) |
Assert.Empty over a .Where() only reports that something was left in the collection; xUnit2029 flags it for that reason. Moving the predicate into Assert.DoesNotContain names the offender instead — the catalog entry no firmware fault backs, or the code claimed twice — which is what the CodesAreUnique comment already asked the failure to say. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.