181 lines
16 KiB
Markdown
181 lines
16 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. |
|
||
| 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. |
|
||
| 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. Needs the udev rule + kernel cmdline persistence described under *Camera acquisition* below. |
|
||
|
||
## 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).
|
||
- **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.
|