193 lines
18 KiB
Markdown
193 lines
18 KiB
Markdown
# Known Issues — Status
|
||
|
||
This tracks the reproduction blockers and robustness issues identified in the original code and what the
|
||
refactor did about them.
|
||
|
||
## Resolved
|
||
|
||
| # | Original issue | Resolution |
|
||
|---|----------------|------------|
|
||
| 1 | Hardcoded config path (`/home/ggs/...`) | Config search order: `--config` → `$FGC_CONFIG` → `./config.ini` → exe dir → XDG ([src/core/Paths.cpp](../src/core/Paths.cpp)) |
|
||
| 2 | Hardcoded image output path | `[Paths] output_dir` with `~`/`$ENV` expansion + sensible default |
|
||
| 3 | Startup scripts with `~/projects/...` | Replaced by path-independent [scripts/run.sh](../scripts/run.sh) + [systemd unit](../scripts/fire-gimbal-control.service) |
|
||
| 4 | Vimba X required to build | `WITH_VIMBA` CMake option (default ON); `OFF` builds a mock-only binary |
|
||
| 5 | No hardware path; exits if MQTT down | Mock implementations + runtime toggles; MQTT failure now logs and continues |
|
||
| 8 | Plaintext MQTT credentials | `$FGC_MQTT_USER`/`$FGC_MQTT_PW` env override; `config.ini` gitignored |
|
||
| 9 | MQTT busy-wait (`while(running);`) | Gone — Paho async client + `set_automatic_reconnect`; no spin thread |
|
||
| 10 | `parser()` missing return | Old parser removed; `parseTelemetryLine` returns `std::optional` cleanly |
|
||
| 11 | Fragile Boost.Spirit command grammar | Replaced by `parseCommand` whitespace tokenizer ([src/core/CommandParser.cpp](../src/core/CommandParser.cpp)) |
|
||
| 12 | Two divergent `config.ini` files | Single committed `config/config.example.ini`; real configs gitignored |
|
||
| 6 | Telemetry field order: humidity before temperature | **Obsolete** — migrated to the current firmware's `ST` protocol (encoder counts, no environmental fields); humidity/temp/fan are gone |
|
||
| 7 | Trigger fires while `is_moving == 1` (not when stopped) | **Fixed** by the protocol migration — capture is now move → **settle** → trigger; the camera fires only once both axes report standstill at the target ([CaptureScheduler.cpp](../src/core/CaptureScheduler.cpp)) |
|
||
|
||
Also added along the way: a leveled logger, typed/validated config, an SDK-independent core library, and a
|
||
doctest unit-test suite (`ctest`).
|
||
|
||
## Open / needs hardware confirmation
|
||
|
||
| # | Issue | Status |
|
||
|---|-------|--------|
|
||
| 13 | `[Motor]` degrees↔counts calibration | The `config.example.ini` values are **placeholders**. Home, then run `gimbal heading setup` (yaw, needs no IMU) and `gimbal pitch setup` (pitch, needs the IMU) on the rig. |
|
||
| 14 | Capture sweep on hardware | **Mechanically verified on the rig (2026-08-07).** A full `--init --start` run homed both axes and completed `MOVE → settle → trigger` for 15/15 waypoints with 0 dropped frames, 0 stalls and 0 acquisition failures. `kSettleTolCounts` (600) and the per-interval timing in [CaptureScheduler.cpp](../src/core/CaptureScheduler.cpp) have **not** been tuned against observed `ST` behaviour yet, and the settle tolerance has not been checked against a sharp image (see #16). |
|
||
| 15 | **Camera profiles unverified on a lit scene** | The `standard`/`night` profiles must be exported from the VimbaX Viewer (see `config/camera_profiles/rgb/README.md`) and have not been measured against real light. Confirm on the rig that `LoadSettings` succeeds, that `standard` sustains 10 fps with `frames_dropped` at 0, and that the auto-exposure lands on a sensible `ExposureTime` outdoors. The only previous rig test ran with the lens covered. **2026-09-24:** tower 121 has still never imaged a scene - a frame grabbed directly and the program's own saved images (2026-09-21) are a smooth grey gradient with no edges, i.e. the lens is covered or grossly out of focus. Tower 111's images from the same afternoon show nothing sharp anywhere either (soft blobs, no edges) - defocus, a cover or a fogged window; unexplained. `test camera` (below) proves the camera and its data path work in both cases; it cannot, by design, judge the picture. |
|
||
| 16 | **`settle_wait_ms` untuned** | `standard` waits 1 s and `night` 2.5 s after standstill before keeping a frame. Both are reasoned (≫ one frame period; long enough for the auto-exposure to see the new scene) but not measured. Check on a lit scene that the kept frame shows no slew blur and that its exposure has actually converged for that heading, and shorten the wait if it has. **Seen in real data 2026-09-24:** of eight consecutive images tower 111 kept at 8 deg/s, one was 98.6 % clipped white, one 57 % black and two about half clipped - frames kept before the continuous auto-exposure had caught up with the new view. |
|
||
| 19 | **USB preconditions do not survive a reboot** | `usbcore.usbfs_memory_mb` reverts to 16 (needs 1000) and USB3 LPM re-enables on the camera's port. **Both were wrong on the LattePanda when tested on 2026-08-07**, and frame delivery stalls without them regardless of application code. **Solved on tower 121 by the setup kit** (`hardware/lattepanda-iota/preconditions/`): a systemd-tmpfiles drop-in for usbfs and two udev rules, all installed by `install.sh --preconditions`. Verified 2026-09-23 — `usbfs_memory_mb` reads 1000 after **6 days of uptime**, so it survived the boot, and both rules are in `/etc/udev/rules.d/`; and again across a deliberate reboot the same day, when `install-tower --verify` read `usbfs_memory_mb` 1000 and the camera port's `usb3_lpm_permit` 0 (a check `test hardware camera` now makes too, once `[Test] usb3_lpm_path` is set - the installer sets it). Note it is **tmpfiles, not the kernel cmdline** the original entry proposed. **On the Delta fleet**: tower 111 read 16 and `u1_u2` on 2026-09-23, and the Delta kit (`hardware/lattepanda-3-delta/`, the same two files) was applied the same day - 1000 and `0` afterwards, confirmed by `test hardware camera`. Confirmed across a reboot on 2026-09-24 (`install-tower --verify`). **Solved on both boards**; open only for Delta towers the kit has not reached. |
|
||
|
||
## Camera acquisition (USB3) — root cause + approach
|
||
|
||
Real-camera capture (Alvium 1800 U-2040c, Sony IMX541, 20.4 MP) was brought up on the LattePanda this
|
||
session. Two problems were root-caused (both **host/USB3-side**, reproduced in Allied Vision's own
|
||
`vmbpy` — the camera keeps acquiring while the host stops receiving; **no kernel errors**):
|
||
|
||
1. **Camera shipped in hardware-trigger mode** (`TriggerSource=Line0`) → produced no frames. Fixed by
|
||
configuring **software trigger** explicitly in-session in [VimbaCameraSource.cpp](../src/camera/VimbaCameraSource.cpp)
|
||
(relying on the camera's persisted user set alone did not work).
|
||
2. **Acquisition stall** — frame delivery stops after a few frames:
|
||
- **USB3 hardware LPM (U1/U2) was enabled** on the camera's link (the camera logs
|
||
`Enable of device-initiated U2 failed`) → silent bulk-transfer stalls under load. Disable per-port:
|
||
write `0` to `/sys/devices/.../usb2/2-0:1.0/usb2-port<N>/usb3_lpm_permit`, then re-enumerate
|
||
(the per-device `power/usb3_hardware_lpm_u*` files are read-only). **Make persistent** via a udev
|
||
rule matched on idVendor `1ab2`.
|
||
- **The host xHCI cannot reliably move large single frames.** Measured (LPM off, paced ~1 fps): RGB8
|
||
full-res **61 MB stalls in 3–5 frames**; **≤ ~20 MB frames sustain when *paced*** (BayerRG8 full-res
|
||
20 MB → 21 frames/20 s; 3.8 MB → 41/20 s). Freerun at max rate stalls even for medium frames.
|
||
- Also raise `usbcore.usbfs_memory_mb` to 1000 (persist via kernel cmdline; resets to 16 on reboot).
|
||
|
||
**Approach (mode-file-driven, see `[Camera]` in [configuration.md](configuration.md)):** RGB8 +
|
||
**2×2 binning** (~5 MP, ~15 MB) keeps de-Bayering **and** white balance on-camera (the on-camera pipeline
|
||
applies white balance before de-Bayering). `DeviceLinkThroughputLimit` is kept conservative (~250 MB/s;
|
||
≥450 caused incomplete frames). Full 20 MP is possible via BayerRG8 (20 MB) + a host de-Bayer step if
|
||
reduced resolution proves insufficient.
|
||
|
||
**Acquisition is a continuous free-running stream** at the active mode's `AcquisitionFrameRate`; the
|
||
scheduler stops at each waypoint, waits `settle_wait_ms`, and keeps the newest frame. This replaced a
|
||
per-waypoint software trigger, which worked on the rig but existed to serve a quality gate that no longer
|
||
does. Free-running is what keeps the camera's own auto-exposure converged, and the frames it discards
|
||
between waypoints cost nothing the scan was going to use.
|
||
|
||
Note the bandwidth ceiling this implies: at 10 Hz, RGB8 and 2×2 binning, the stream is ~153 MB/s — safely
|
||
inside the 250 MB/s limit, but full resolution at that rate is not reachable at all (61 MB/frame). See
|
||
*Bandwidth* in [architecture.md](architecture.md#image-capture).
|
||
|
||
**Status (rig-tested 2026-08-07, before the mode rework).** The measurements below were taken with the
|
||
older software-trigger path; they bound **frame size and pacing**, which the change does not alter.
|
||
|
||
**Confirmed working** — a full sweep at 15/15 waypoints with **0 dropped, 0 stalls, 0 acquisition
|
||
failures**, across four runs. Camera configuration was verified *independently* through Allied Vision's own
|
||
`vmbpy` (`RGB8`, binning 2, 2256×2256), and `ExposureTime` writes land within quantisation (`want 20000 →
|
||
readback 19980.6`) — which is also the evidence that feature writes reach the device, the mechanism every
|
||
mode file now relies on. Real-motor integration and clean shutdown (camera not left wedged) verified.
|
||
|
||
**Not confirmed** — the lens was covered for the whole test, so every frame was black (mean ~1/255).
|
||
Everything that depends on seeing a real scene is still open: see **#15–#16** in *Open / needs hardware
|
||
confirmation* above, plus **#19** for the USB preconditions.
|
||
|
||
A useful diagnostic for the "is it the optics or the code?" question: with frame status validated and gain 0,
|
||
a **2-second** exposure produced mean 3.6/255 (brightest pixel 80). A lit room at gain 0 should be well
|
||
exposed near 10–30 ms, so that is ~1000× too little light — conclusively optical, not software. `vmbpy`
|
||
scripts for this are worth keeping; note that wrapping them in `timeout` SIGTERMs Python and skips
|
||
`stop_streaming()`, which leaves the camera unusable until the next USB reset.
|
||
|
||
## Verification caveats
|
||
|
||
- **Full build verified on the LattePanda**: a `WITH_VIMBA=ON WITH_MQTT=ON` build compiles and links on the
|
||
device (real Vimba X SDK + Paho fetched). The MQTT wrapper also builds on the dev box (GCC 16 / CMake 4 — see
|
||
`cmake/Paho.cmake` for the toolchain-compat shims).
|
||
- **Serial protocol verified live against real firmware** (LattePanda, this session): `ENABLE`/`HOME`/`SPEED`
|
||
reach the firmware, the `ST` telemetry parses with zero unparsed lines across a full session, and a live
|
||
`--init` drove a clean re-home of both axes to `READY`.
|
||
- **Real-camera capture verified on the rig (2026-08-07)**: a live `--init --start` run with the real Alvium
|
||
and real motor captured 15/15 waypoints, one software-triggered frame each, 0 dropped and 0 stalls. What
|
||
this does **not** cover is image *content* — the lens was covered, so exposure/blur behaviour on a real
|
||
scene remains open (#15–#17).
|
||
- **Log rotation verified on the rig (2026-09-23); pruning is not.** With
|
||
`[Logging] file_max_mb = 1` on tower 121, a segment closed at 1048513 bytes against
|
||
the 1048576 cap — rotating *before* the write that would cross it, on a whole line —
|
||
and the next segment re-emitted the header. Retention (`file_keep`) needs a fourth
|
||
segment to observe a deletion, roughly three hours at that cap, and was not run.
|
||
`tests/test_filelogsink.cpp` covers it directly.
|
||
- **The positive-travel guard has never fired in anger.** `checkTravelFitsPositive()`
|
||
catches a `center_count` smaller than half the homed travel, which puts the negative
|
||
end below zero. It was tested against real numbers and shipped *after* 121's centre
|
||
was raised to 1000000, so no tower has ever tripped it. Whether the TMC5160 actually
|
||
misbehaves below zero is still not established — see the note in
|
||
`include/fgc/HardwareChecks.h`.
|
||
- **Makefile parity** is moot: the Makefile was removed in favour of CMake, and the full
|
||
`WITH_VIMBA=ON WITH_MQTT=ON` CMake build now runs on the device.
|
||
- **Demo mode** copies `bin/x64/Release/test_smoke.jxl`, resolved relative to the working directory. Run from a
|
||
directory where that path exists, or extend `ImagePipeline::Params::demo_image`.
|
||
|
||
## Possible follow-ups (not done)
|
||
|
||
- **A retained `Env` payload may still sit on the broker.** `GGS/FWT/<tower>/Env` was
|
||
published retained until it was removed, so a broker that saw the old build still holds
|
||
the last ambient reading and hands it to every new subscriber — a frozen temperature and
|
||
humidity with an old timestamp, indistinguishable from a sensor that stopped reporting.
|
||
Nothing in the code does this any more, so nothing here would otherwise explain it.
|
||
Clear it broker-side by publishing a zero-length retained message to that topic.
|
||
|
||
- **NWU vs ENU is decided by a constant, not by the device.** `kMtiReference`
|
||
([MtiProtocol.h](../include/fgc/MtiProtocol.h)) says the MTi reports against NWU, and the
|
||
host re-references it to NED. The two candidates give *identical* roll and pitch and
|
||
differ only in yaw, by exactly 90°, so nothing in the software can tell them apart —
|
||
only a compass check on the rig can. If corrected yaw is 90° off, change that constant to
|
||
`ImuReference::Enu`. Reading the device's actual coordinate-system bits would settle it,
|
||
but their position in `OutputSettings` is not established for this firmware.
|
||
- **`[IMU] x` should be re-confirmed now that the reference is corrected.** `x = 180` was
|
||
set while pitch was inverted. Level the gimbal and confirm roll and pitch ≈ 0, tip the
|
||
nose up and confirm pitch goes **positive**, roll right and confirm roll goes positive.
|
||
Then re-run `gimbal pitch setup` — its reference moved with the earth-frame correction.
|
||
Yaw is unaffected: `gimbal heading setup` uses landmarks, not the IMU.
|
||
- **The handshake still does not await acks for its state transitions.**
|
||
`GoToConfig`/`GoToMeasurement` are blind writes separated by 60 ms sleeps
|
||
([MtiImuSource.cpp](../src/serial/MtiImuSource.cpp)). The readback verification makes
|
||
this harmless for correctness — if `GoToConfig` is missed the Sets do nothing and the
|
||
mismatch is reported — but a missed transition is diagnosed one step later than it
|
||
happened.
|
||
- **A saved pitch calibration silently predates the IMU mount.** `gimbal pitch setup` fits IMU pitch
|
||
against encoder counts, so a calibration made before `[IMU] x/y/z` were set — or under different
|
||
values — has that mount error baked into `pitch_angle_at_center`. Nothing detects this; re-run
|
||
`gimbal pitch setup` after changing the mount. Stamping the mount into the saved `[Motor]` block and
|
||
warning on a mismatch would close it. Yaw no longer has this exposure at all, since
|
||
`gimbal heading setup` never consults the IMU.
|
||
- **The mount is host-side only.** The rotation is applied to the MTi's output, so the XKF filter itself
|
||
still runs in the sensor frame. Xsens `SetAlignmentRotation` (MID 0xEC) would push the alignment into
|
||
the device so the filter's own gravity/magnetic corrections are made in the gimbal frame — better
|
||
filtering, at the price of writing to the unit's non-volatile memory, which `config.ini` cannot undo.
|
||
|
||
- **A profile cannot be reloaded without a restart.** `set camera profile <name>` re-applies a profile from
|
||
the copy loaded at start-up, so re-exporting the XML while the program runs has no effect. Tuning on the
|
||
rig therefore costs a restart per export. Re-reading on switch would fix it, at the price of a failure
|
||
mode (a mid-session unreadable export) that start-up loading currently rules out.
|
||
- **Nothing checks that a profile actually took.** `LoadSettings` reports success for the file as a whole,
|
||
but the camera is never read back to confirm the resulting state. A post-apply readback of a handful of
|
||
key features (`AcquisitionFrameRate`, `Width`, `PixelFormat`) would turn a silent misconfiguration into
|
||
an error.
|
||
- Graceful shutdown on SIGINT (currently exit via `exit`/Ctrl-D; a pending `getline` can delay shutdown).
|
||
- Support a second camera backend. `driver` is parsed and validated but only `vimba` is accepted, so a
|
||
thermal or NIR unit needs a new `ICameraSource` implementation behind that key.
|
||
- Reintroduce optional image upload to the ground station, config-driven (the old hardcoded NFS/SMB upload was
|
||
removed).
|
||
- **Calibration persistence needs the TUI.** `gimbal pitch setup` and `gimbal heading fit` are always
|
||
applied to the live session (display, manual moves, MQTT bearings, **and** the capture scheduler) and
|
||
logged. In the **TUI**, the activity strip then prompts `Save … as the default? (y/n)` — `y` writes
|
||
the `[Motor]` map into `config.ini`, `n` keeps it session-only. In the **headless** console there is
|
||
no prompt, so it stays session-only there; copy the logged values into `[Motor]` by hand.
|
||
`gimbal heading offset` is the exception — it saves immediately, with no prompt.
|
||
- **`gimbal nudge`** moves a fixed fraction of the **homed** endstop-to-endstop travel (from the
|
||
firmware dump), so it is independent of the degrees↔counts calibration. If no dump has been captured
|
||
yet (not homed / no `gimbal dump`), nudge requests one and does nothing that press — it never falls
|
||
back to a degree-derived guess. `gimbal jog` is the raw-counts equivalent when a finer step is
|
||
wanted.
|
||
- Ambient temperature/humidity (SHT41, `IEnvSensor`/`Sht41EnvSensor`/`MockEnvSensor`) is fully
|
||
integrated and **validated against the physical sensor on the LattePanda** — config-gated
|
||
(`[Features] enable_env`, `[Env]`), Sensors panel + expanded `i` view + MQTT `Env` topic wired.
|
||
It is **off by default** (`enable_env = false`), so a config without an `[Env]` section shows the
|
||
panel as `pending` — that status means "no driver constructed", not "sensor unreachable"; an
|
||
enabled-but-silent sensor reads `no fix` instead. Dev machines without the hardware should use
|
||
`--mock-env` (implies `enable_env`) rather than editing config.
|
||
- `[Env] i2c_addr` used to be parsed with a decimal-only `std::stoi`, so the `0x44` form every
|
||
datasheet and `i2cdetect` uses silently became address `0x00` and the sensor never answered. It now
|
||
accepts decimal or `0x` hex (`getIntAutoBase` in `Config.cpp`, covered by a test). Other integer
|
||
config keys stay decimal-only on purpose, so a leading zero never turns into octal.
|