[1.4] Fix Navigator not being detected on the first boot of a Bookworm install - #4232
Conversation
Automated PR Review0. Summary
Backports to No further comments, nice job 👍 Generated by PR Review Bot. This is advisory, a human reviewer must still approve. |
a841622 to
990ead9
Compare
4895884 to
68c3190
Compare
dac735b to
fa49b37
Compare
2665849 to
bdab005
Compare
|
Using the Raspbian image built from this branch (from the HACK commit, now ditched, but artifacts still available here), I burned the SD cards for each device and booted them up. Aside from what the LLM verified by accessing them via SSH, I manually and independently tested and verified myself:
Functionally, I believe this is ready. @patrickelectric can you check whether we are missing anything? |
bdab005 to
48e3e83
Compare
48e3e83 to
5e808b0
Compare
|
ping @patrickelectric |
cac27ce to
e6b62d4
Compare
Bookworm moved the boot partition to /boot/firmware and left plain text stubs behind at /boot/config.txt and /boot/cmdline.txt, so the Pi4 and BCM28XX scripts were appending the Navigator overlays to files the firmware never reads. A Pi4 installed over Bookworm came up with no I2C, SPI or UART configured, and the Navigator went undetected. The dtc output path was hardcoded to /boot/overlays as well. On Bookworm that only resolves because the boot partition migration leaves a /boot/overlays symlink behind, which no package owns, so the Navigator LED overlay was one missing compatibility link away from landing outside the boot partition. Resolve the boot partition once per script and abort when neither layout is mounted, rather than configuring a stub and reporting success. A Pi5 is newer than Bullseye, so its script only ever accepts /boot/firmware.
A missing board section was appended as a single "\n[pi4]" entry, which the section lookup can never match again because re.match anchors at the start of the string. Every call appended yet another header, and the start index pointed one past the list, so the section bounds check in the conflict filter could never hold and each configuration line was deleted right after being inserted. A first pass over a stock config.txt left 30 empty [pi4] sections and no overlays, so a fresh install only got them two reboots later, once the saved file had been split back into lines.
The section fell back to [pi5] for every board that is not a Pi4, and a board filter the firmware does not recognise is applied rather than ignored, so on anything older than a Pi5 that configuration would have reached the pins. Nothing gets here on a Pi3 today, main() only runs this patch for a Pi4 and a Pi5, but a board detection that once went wrong is exactly what its sibling revert_update_dwc2 exists to undo. Guard it the way update_navigator_overlays already does.
A board install script that writes to the /boot stub instead of the boot partition still exits 0, so the built image is the only place the mistake shows. Check that the image carries the board section, the Navigator overlays, the memory cgroup docker needs, and no serial console holding the autopilot port.
The startup patches and the board install scripts have to agree on what a Navigator needs, and both had drifted into writing configuration where the firmware never reads it. Cover the two boot layouts BlueOS ships on: Bullseye, whose boot partition is at /boot and whose config.txt already carries a [pi4] section in the middle of the file followed by a second [all], and Bookworm, at /boot/firmware and with no board section to patch at all. Both baselines are the stock files of the images BlueOS builds from, so the result can be checked against hardware, and patching the Bullseye one reproduces the [pi4] section of a Pi4 that has been running BlueOS on Bullseye. A Navigator cannot be used on a Pi3, its pinout differs, and for the same reason the overlays of a Pi4 and a Pi5 are not interchangeable. Both are pinned down too: a Pi3 has to come out of the patches untouched, and neither board may end up carrying the other's overlays. The build time image check only runs on the upstream repository, where nobody reads it until a release ships, so its expectations are pinned here to what the install scripts actually write.
The install scripts now refuse to configure the plain text stub Bookworm leaves at /boot once the boot partition moved to /boot/firmware, but the runtime repair path did not. locate_file returns the first candidate that exists, so on a Bookworm host whose boot partition is not mounted it settled on /boot/config.txt, the stub, and the patches wrote the whole board section into a file the firmware never reads, rebooted, and from then on found their own work in place and converged into a permanently unconfigured state with no error. Refuse the stub by the same DO NOT EDIT THIS FILE marker the install scripts key on, so both halves of the contract agree, and let the existing config_file None guards skip the patch loudly rather than write nowhere.
e6b62d4 to
1349efc
Compare
| printf ' %s\n' "${MISSING[@]}" | ||
| echo "The board install script did not configure the image's boot partition." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
just to be sure.. have you tested this in the pimod ci machine ?
Fixes the Pi4 Bookworm first-boot Navigator detection failure reported in #4230, the underlying reason the startup patches could not recover from it on any board, and the damage it left on devices that already ran the affected release.
Two independent bugs
The install scripts wrote to the wrong file. Bookworm moved the boot partition to
/boot/firmwareand left plain text stubs at/boot/config.txtand/boot/cmdline.txt.install/boards/bcm_27xx.shhardcoded the old paths, so a Pi4 installed over Bookworm got its whole Navigator overlay set appended to a file the firmware never reads. Only those two are real files:/boot/overlaysis a symlink into the boot partition, so the compiled overlay did land there, and1.4.4-beta.20shipsoverlays/spi0-led.dtbonext to a stockconfig.txtwith nothing to load it.bcm_2712.sh(Pi5) already used/boot/firmwarefor its writes, which is why only Pi4 was reported, but it too reached the overlays directory through that symlink, which no package owns.bcm_28xx.shhad the same problem as the Pi4 script. All three now resolve the boot partition the same way.The startup patch that should have repaired that was a no-op on its first run.
boot_config_get_or_append_section()appended a missing board section as a single"\n[pi4]"list entry. The section lookup usesre.match, which anchors at offset 0 and therefore never matches a string starting with a newline, so every call appended another header. The start index was also set one past the end of the list, which made thesection_start < i < section_endguard in the conflict filter unsatisfiable, so each configuration line was deleted immediately after being inserted.A first pass over a stock
config.txtproduced 30 empty[pi4]sections and zero overlays. The overlays only landed on a later boot, once the saved file had been split back into lines, and sinceBOOT_LOOP_DETECTORskips the boot right after a reboot, that took three boots. This part was never Pi4-specific; Pi5 was only saved by its install script writing the right file.Repairing devices that already ran the affected release
Every device that booted
1.4.4-beta.20carries roughly 30 stray[pi4]headers. Two consequences, both fixed here:boot_config_merge_duplicated_sections()now folds them into a single section on the next boot, keeping every user line in the order the firmware applies it, and leaves aconfig.txtwith one section byte-identical so it does not ask for a needless reboot. Order matters because a duplicate header reopens the same filter rather than starting a section only the first of which is read: hoisting the strays above the survivor's own lines would have flipped the effective value of any key set under both headers, on exactly the devices this repairs.install.shon such a device removed its Navigator configuration and reported success.line_number=$(grep -n "\[pi4\]" ... )returns one line per match, so$line_numberbecame multi-line, every insertionsedfailed withunknown command, and the deletion loop had already stripped the overlays. It now takes the first match only.Making the failures loud
Both halves of the original bug shared a failure mode: writing to the wrong place still exits 0.
/boot/firmware/config.txtwins;/boot/config.txtis used only when it is a real config and not the Bookworm stub (identified byDO NOT EDIT THIS FILEbeing its whole first line); otherwise the script prints why and exits 1. A Pi5 is newer than Bullseye, sobcm_2712.shaccepts nothing but/boot/firmware: boot files at/booton that board are never its boot partition. Previously an unmounted boot partition silently configured a stub, and for the Pi5 that was a regression in failure mode, since hardcoding/boot/firmwareat least madesedfail.bcm_27xx.shandbcm_28xx.shnowset -e, whichbcm_2712.shalready had. Every command in both was audited and both were run end to end against fake roots to confirm nothing that legitimately fails now aborts an install.blueos_startup_update.pylocatedconfig.txt/cmdline.txtwith a first-matchfind, so on a Bookworm host whose boot partition was not mounted it settled on the/bootstub, wrote the whole board section into a file the firmware never reads, rebooted, and from then on found its own work in place and converged into a permanently unconfigured state with no error. It now checks the sameDO NOT EDIT THIS FILEmarker the install scripts key on and, on the stub, logs and skips the config patches through the existingconfig_file is Noneguards rather than writing nowhere. The marker has to be the file's whole first line, so a realconfig.txtthat carries the phrase in a comment keeps being patched: refusing one would leave a correctly mounted board with no overlays, no cgroups and no OTG ethernet while still reporting success. It reads that line withrun_commandrather thanload_file, because this runs before therun_command_is_working()check andload_fileraises when the host connection is not up yet, which would take every patch of the boot down with it, including the ones this PR is not about.Other bugs found on the way
update_dwc2()picked its section as"pi4" if get_cpu_type() == CpuType.PI4 else "pi5", so every board that is not a Pi4 got a[pi5]section, and a board filter the firmware does not recognise is applied rather than ignored. Nothing reached a Pi3 in the field, sincemain()only runs this patch for a Pi4 and a Pi5, but a board check that once went wrong is exactly why its siblingrevert_update_dwc2()exists. It now handles Pi4 and Pi5 explicitly and refuses anything else.boot_cmdline_add_modules()used a falsy index check, so whenmodules-load=was the first word ofcmdline.txtthe merged line was never written and a second, deadmodules-load=was appended instead.dtoverlay=i2c3-pi5,baudrate=400000anddtoverlay=i2c3-pi5.baudrate=400000, where the.matches the comma variant, so keeping both spellings depended on undocumented list order.boot_config_remove_section()on an absent section appended the header, deleted it again, and reported a change, requesting a reboot for a section that never existed.Guardrails
core/tools/blueos_startup_update/test_blueos_startup_update.py(54 tests) runs the patches against the authentic stockconfig.txtandcmdline.txtof both the Bullseye and the Bookworm base images, for every board we configure, and pins:# customprotected lines survivebashagainst four fake-root layouts, resolves correctly and exits non-zero when it cannot tell, down to the Pi5 script refusing a/bootlayoutconfig.txtandcmdline.txt, and still accepts a real Bullseye/bootand a mountedconfig.txtwhose comments happen to carry the markermain()itself resolves both boot files through that guard, so reverting the one line that wires it in fails the suite instead of quietly restoring the original bugBOOT_PATH, anywhere, outside the probe that resolves itEvery guard was checked by reverting the fix it protects and confirming the suite goes red, including the shell ones.
The image build now asserts that the boot partition of the image it just produced really carries the board configuration: in
config.txtthe board section and the Navigator overlays, and incmdline.txtthe memory cgroup docker needs and the removal of the serial console that would hold the autopilot port.cmdline.txtis reached through a path of its own and was lost the same way — the shipped1.4.4-beta.20Bookworm image still carries stockconsole=serial0,115200and no cgroup parameters. Run against that image, this check fails on every line it looks for, which is the bug sitting in a released artifact.That check only ever runs upstream, so
test_image_build_checks_what_the_install_scripts_writeties each line it looks for back to the install script that writes it. A check that stopped matching the scripts fails the test suite instead of going unnoticed until a release.Out of scope
Three problems found while working on this, none of them fixed here:
get_cpu_type()does not recognise it.deploy/pimod/blueos.Pifilerunstouch /boot/ssh, which on Bookworm lands on the rootfs stub directory rather than the boot partition.Test plan
Four combinations to cover: Pi4 Bullseye, Pi4 Bookworm, Pi5 Bookworm and Pi3 Bullseye. Three images cover them, and all three were built from this branch:
arm-v7 bullseyefor the Pi3 and the Pi4,arm-v7 bookwormfor the Pi4, andarm64-v8 bookwormfor the Pi5. They came from266584936, a temporary commit that opened the image build up to pull requests while writing nothing to any registry, and which differs from the current head only in that build plumbing. It has since been dropped, so the branch is the fix alone.Each artifact was read before burning: its FAT boot partition was opened with
mtoolsand checked against expectations parsed out of the install scripts. The same check reports 5 problems against1.4.4-beta.20.The Pi5 probe was narrowed to
/boot/firmwareafter those images were built, so the probe of all three board scripts was run again, read only, on four live boards: two Pi5 Bookworm, a Pi4 Bookworm and a Pi4 Bullseye. Each resolved the boot partition its own board actually uses,/boot/config.txtis a stub on all three Bookworm boards, andbcm_2712.shrefused the Pi4 Bullseye layout.Every image carries
blueos-core:1.4-dev, so the boot files come from the fixed install scripts while the patches running on the first boot are the old ones. That is what makes a first boot a test of the install scripts on their own, and it held: on both Bookworm boards the first boot asked to restart fordns,noIPV6,swap,wpaandnetworkmanager, and for no boot configuration patch, because there was nothing left to fix. The fixed patches were then deployed to each board and run twice.Pi4 Bullseye — Pi 4B Rev 1.2,
arm-v7 bullseye pi4BOOT_PATHresolved to/bootand the configuration went into the[pi4]section the Bullseye image already ships, leaving[cm4],[all],[pi4],[all]— no second board section, and the two stock[all]sections left alonecgroup_enable=memoryis set,console=serialis gone, andi2c-4,i2c-6,spidev0.0,spidev1.0-1.2andttyAMA0-3are all presentDetected Linux board: NavigatorPi4,Using Navigator flight-controllerconfig.txtorcmdline.txtand no restart requested, on either of two runs[pi4]headers with user configuration under one of the strays collapsed to a single section with that line inside it, and a second run changed nothing and asked for no rebootPi4 Bookworm — Pi 4B Rev 1.5,
arm-v7 bookwormBOOT_PATHresolved to/boot/firmware,spi0-led.dtbowas compiled into/boot/firmware/overlays, and[pi4]carries the whole seti2c-4,i2c-6,spidev0.0,spidev1.0-1.2andttyAMA3/4/5present,cgroup_enable=memoryset,console=serialgoneDetected Linux board: NavigatorPi4,Using Navigator flight-controllerconfig.txtorcmdline.txtand no restart requested, on either of two runs/boot/firmwareunmounted,bcm_27xx.shprinted why and exited 1, leaving the/bootstub untouched, and that status carries:configure_board.shends on the board script's pipeline andinstall.shruns underset -e, so the install stops there rather than reporting successPi5 Bookworm — Pi 5B Rev 1.0,
arm64-v8 bookworm pi5BOOT_PATHresolved to/boot/firmware, the overlay was compiled into/boot/firmware/overlays, and[pi5]carries the Pi5 set with bothi2c3-pi5spellingsi2c-3,i2c-6,spidev0.0,spidev1.0-1.2andttyAMA0/2/3/4/10present,cgroup_enable=memoryset,console=serialgoneDetected Linux board: NavigatorPi5Pi3 Bullseye — Pi 3B,
arm-v7 bullseye pi4This board booted the image three times and came all the way up, but it never got a DHCP lease on
eth0and lost its hotspot a minute in to #4233, so it stayed unreachable and was read back from its SD card rather than over ssh.[all],[all]: the[cm4]and[pi4]sections the Pi4 image ships are gone, and noi2c,spi,uartordwc2overlay line survives anywhere inconfig.txtmodules-load=dwc2,g_etherwas taken back out ofcmdline.txtrevert_update_dwc2andclean_config_pi3and rebooted once, and the boot after that applied nothing and asked for no restartconfig.txtbyte-identical to the one the shipped patches left on the boardbcm_28xx.shcannot be reached through an image:configure_board.shpicks the board script from the running model, and the Bullseye image is built on a Pi4, so a Pi 0-3 only runs that script wheninstall.shis executed on it directly. The suite covers it by running it under realbashagainst fake roots.