[1.4] Fix Navigator not being detected on the first boot of a Bookworm install

.github/workflows/test-and-deploy.yml

@@ -303,6 +303,20 @@ jobs:
 
       - name: Add /boot additions
         run: |
+          # configure_board.sh dispatches on the host device tree, so the board configuration the image
+          # must end up with is decided by the runner's own hardware, not by the image it builds.
+          case "${{ matrix.runner }}" in
+            pi5-builder)
+              EXPECTED=("[pi5]" "dtoverlay=i2c3-pi5,baudrate=400000" "dtoverlay=spi1-3cs")
+              ;;
+            blueos-ci|pi4-builder2)
+              EXPECTED=("[pi4]" "dtoverlay=i2c4,pins_6_7,baudrate=1000000" "dtoverlay=spi1-3cs")
+              ;;
+            *)
+              echo "Unknown runner ${{ matrix.runner }}, add the board configuration its image must contain."
+              exit 1
+              ;;
+          esac
           sudo apt-get update && sudo apt-get install -y parted kpartx
           # Create mount point if it doesn't exist
           sudo mkdir -p /mnt/piboot
@@ -312,8 +326,27 @@ jobs:
           sudo mount "/dev/mapper/${LOOP_DEVICE}p1" /mnt/piboot
           # Create ssh and userconf files
           sudo cp install/boards/config.toml /mnt/piboot/custom.toml
+          # A board install script writing to the /boot stub instead of the boot partition still exits 0,
+          # so the image itself is the only place where a missing Navigator configuration can be caught.
+          MISSING=()
+          for LINE in "${EXPECTED[@]}"; do
+            sudo grep -qxF "$LINE" /mnt/piboot/config.txt || MISSING+=("config.txt: $LINE")
+          done
+          # cmdline.txt is reached through its own path and can be left behind on its own. Whatever
+          # board this is, the memory cgroup docker needs is enabled and the serial console that
+          # would hold the autopilot port is dropped.
+          sudo grep -q "cgroup_enable=memory" /mnt/piboot/cmdline.txt || MISSING+=("cmdline.txt: cgroup_enable=memory")
+          if sudo grep -q "console=serial" /mnt/piboot/cmdline.txt; then
+            MISSING+=("cmdline.txt: console=serial is still there")
+          fi
           sudo umount /mnt/piboot
           sudo kpartx -d deploy/pimod/blueos.img
+          if [ ${#MISSING[@]} -ne 0 ]; then
+            echo "Missing from the boot partition of the ${{ matrix.os }} ${{ matrix.platform }} image built on ${{ matrix.runner }}:"
+            printf '  %s\n' "${MISSING[@]}"
+            echo "The board install script did not configure the image's boot partition."
+            exit 1
+          fi
           echo "Boot partition updated successfully."
 
       - name: Sanitize platform name

core/tools/blueos_startup_update/blueos_startup_update.py

@@ -99,8 +99,8 @@ def boot_config_get_or_append_section(config_content: List[str], section_name: s
         (i for (i, line) in enumerate(config_content) if re.match(section_match_pattern, line, regex_flags)), None
     )
     if section_start_line_number is None:
-        config_content.append(f"\n[{section_name}]")
-        section_start_line_number = len(config_content)
+        config_content.extend(["", f"[{section_name}]"])
+        section_start_line_number = len(config_content) - 1
 
     any_section_match_pattern = r"^\[.*\].*$"
     section_end_line_number = next(
@@ -117,20 +117,62 @@ def boot_config_get_or_append_section(config_content: List[str], section_name: s
     return (section_start_line_number, section_end_line_number)
 
 
+def boot_config_merge_duplicated_sections(config_content: List[str], section_name: str) -> None:
+    regex_flags = re.IGNORECASE | re.DOTALL | re.MULTILINE
+
+    section_match_pattern = r"^\[" + re.escape(section_name) + r"\].*$"
+    any_section_match_pattern = r"^\[.*\].*$"
+
+    # A board filter is applied until the next one, so config.txt is a run of sections, each starting
+    # at its header, with the configuration that applies to every board ahead of the first header
+    sections: List[List[str]] = [[]]
+    for line in config_content:
+        if re.match(any_section_match_pattern, line, regex_flags):
+            sections.append([])
+        sections[-1].append(line)
+
+    duplicated = [
+        index
+        for (index, section) in enumerate(sections[1:], start=1)
+        if re.match(section_match_pattern, section[0], regex_flags)
+    ]
+    if len(duplicated) < 2:
+        return
+
+    # A duplicate header just reopens the same filter, so every stray's configuration is already in
+    # force, and the firmware applies whatever comes last. The strays are emptied into the end of the
+    # first section's run of non-blank lines, which keeps that order and keeps them where the helpers
+    # above, that read a section as ending at the first blank line, will find them
+    (survivor, *strays) = duplicated
+    body_end = next((index for index, line in enumerate(sections[survivor]) if index > 0 and line == ""), None)
+    if body_end is None:
+        body_end = len(sections[survivor])
+    sections[survivor][body_end:body_end] = [line for index in strays for line in sections[index][1:] if line != ""]
+    for index in strays:
+        sections[index] = []
+
+    config_content[:] = [line for section in sections for line in section]
+
+
 def boot_config_add_configuration_at_section(config_content: List[str], config: str, section_name: str) -> None:
     regex_flags = re.IGNORECASE | re.DOTALL | re.MULTILINE
 
     (section_start, section_end) = boot_config_get_or_append_section(config_content, section_name)
 
     section_content = config_content[section_start:section_end]
     config_already_exists = any(
-        section_content for section_content in section_content if re.match(config, section_content, regex_flags)
+        section_content
+        for section_content in section_content
+        if re.match(re.escape(config), section_content, regex_flags)
     )
     if not config_already_exists:
         config_content.insert(section_start + 1, config)
 
 
 def boot_config_remove_section(config_content: List[str], section_name: str) -> None:
+    if section_name not in boot_config_get_available_section(config_content):
+        return
+
     (section_start, section_end) = boot_config_get_or_append_section(config_content, section_name)
     del config_content[section_start:section_end]
 
@@ -168,7 +210,7 @@ def boot_config_filter_conflicting_configuration_at_section(
                 # ...except...
                 and not (
                     # ...if it's the correct one....
-                    re.match(f"^{config}.*$", line, regex_flags)
+                    re.match(f"^{re.escape(config)}.*$", line, regex_flags)
                     # ...and lives inside the correct section.
                     and (section_start < i < section_end)
                 )
@@ -206,7 +248,7 @@ def boot_cmdline_add_modules(cmdline_content: List[str], config_key: str, desire
             cmdline_content.remove(cmdline_content[config_index])
 
     # Replace the first configs line with the combined, append if none
-    if first_config_line:
+    if first_config_line is not None:
         cmdline_content[first_config_line] = f"{config_key}=" + ",".join(desired_config)
     else:
         config_line = f"{config_key}=" + ",".join(desired_config)
@@ -354,7 +396,16 @@ def update_dwc2() -> bool:
 
     # Add dwc2 overlay in pi4 section if it doesn't exist
     dwc2_overlay_config = "dtoverlay=dwc2,dr_mode=otg"
-    section_name = "pi4" if get_cpu_type() == CpuType.PI4 else "pi5"
+    cpu_type = get_cpu_type()
+    if cpu_type == CpuType.PI4:
+        section_name = "pi4"
+    elif cpu_type == CpuType.PI5:
+        section_name = "pi5"
+    else:
+        # A board filter the firmware does not know is applied instead of ignored, so a [pi5]
+        # section would reach the pins of every board older than the Pi5
+        logger.error("Unsupported CPU type for dwc2 update")
+        return False
     boot_config_add_configuration_at_section(config_content, dwc2_overlay_config, section_name)
 
     # Remove any unprotected and conflicting dwc2 overlay configuration
@@ -443,6 +494,10 @@ def update_navigator_overlays() -> bool:
         logger.error("Unsupported CPU type for navigator overlays update")
         return False
 
+    # Devices patched by a release that appended a board section on every boot accumulated strays,
+    # and only the first of them would ever be configured
+    boot_config_merge_duplicated_sections(config_content, section_name)
+
     navigator_configs_with_match_patterns.reverse()
 
     for (config, config_match_pattern) in navigator_configs_with_match_patterns:

install/boards/bcm_2712.sh

@@ -8,8 +8,17 @@ VERSION="${VERSION:-master}"
 GITHUB_REPOSITORY=${GITHUB_REPOSITORY:-bluerobotics/BlueOS}
 REMOTE="${REMOTE:-https://raw.githubusercontent.com/${GITHUB_REPOSITORY}}"
 ROOT="$REMOTE/$VERSION"
-CMDLINE_FILE=/boot/firmware/cmdline.txt
-CONFIG_FILE=/boot/firmware/config.txt
+# Bookworm moved the boot partition to /boot/firmware and left a plain text stub at /boot that
+# configures nothing. A Pi5 is newer than Bullseye, so it never uses the old /boot layout.
+if [ -f /boot/firmware/config.txt ]; then
+    BOOT_PATH=/boot/firmware
+else
+    echo "No boot partition at /boot/firmware, is it mounted?" >&2
+    echo "Refusing to configure a file the board would never read." >&2
+    exit 1
+fi
+CMDLINE_FILE="$BOOT_PATH/cmdline.txt"
+CONFIG_FILE="$BOOT_PATH/config.txt"
 alias curl="curl --retry 6 --max-time 15 --retry-all-errors --retry-delay 20 --connect-timeout 60"
 
 # Download, compile, and install spi0 mosi-only device tree overlay for
@@ -18,7 +27,7 @@ echo "- compile spi0 device tree overlay."
 DTS_PATH="$ROOT/install/overlays"
 DTS_NAME="spi0-led"
 curl -fsSL -o /tmp/$DTS_NAME $DTS_PATH/$DTS_NAME.dts
-dtc -@ -Hepapr -I dts -O dtb -o /boot/overlays/$DTS_NAME.dtbo /tmp/$DTS_NAME
+dtc -@ -Hepapr -I dts -O dtb -o "$BOOT_PATH/overlays/$DTS_NAME.dtbo" /tmp/$DTS_NAME
 
 # Remove any configuration related to i2c and spi/spi1 and do the necessary changes for navigator
 echo "- Enable I2C, SPI and UART."
@@ -41,7 +50,7 @@ if ! grep -q "\[pi5\]" $CONFIG_FILE; then
 fi
 # find the line number of the [pi5] tag
 
-line_number=$(grep -n "\[pi5\]" $CONFIG_FILE | awk -F ":" '{print $1}')
+line_number=$(grep -n "\[pi5\]" $CONFIG_FILE | head -n 1 | awk -F ":" '{print $1}')
 echo "Line number of [pi5] tag: $line_number"
 
 

install/boards/bcm_27xx.sh

@@ -1,13 +1,26 @@
 #!/usr/bin/env bash
 
+set -e
+
 echo "Configuring BCM27XX board (Raspberry Pi 4).."
 
 VERSION="${VERSION:-master}"
 GITHUB_REPOSITORY=${GITHUB_REPOSITORY:-bluerobotics/BlueOS}
 REMOTE="${REMOTE:-https://raw.githubusercontent.com/${GITHUB_REPOSITORY}}"
 ROOT="$REMOTE/$VERSION"
-CMDLINE_FILE=/boot/cmdline.txt
-CONFIG_FILE=/boot/config.txt
+# Bookworm moved the boot partition to /boot/firmware and left plain text stubs behind at /boot,
+# so writing to /boot there configures nothing.
+if [ -f /boot/firmware/config.txt ]; then
+    BOOT_PATH=/boot/firmware
+elif [ -f /boot/config.txt ] && ! head -n 1 /boot/config.txt | grep -qx "DO NOT EDIT THIS FILE"; then
+    BOOT_PATH=/boot
+else
+    echo "No boot partition at /boot/firmware, and /boot is missing or holds the Bookworm stub." >&2
+    echo "Refusing to configure a file the board would never read." >&2
+    exit 1
+fi
+CMDLINE_FILE="$BOOT_PATH/cmdline.txt"
+CONFIG_FILE="$BOOT_PATH/config.txt"
 alias curl="curl --retry 6 --max-time 15 --retry-all-errors --retry-delay 20 --connect-timeout 60"
 
 # Download, compile, and install spi0 mosi-only device tree overlay for
@@ -16,7 +29,7 @@ echo "- compile spi0 device tree overlay."
 DTS_PATH="$ROOT/install/overlays"
 DTS_NAME="spi0-led"
 curl -fsSL -o /tmp/$DTS_NAME $DTS_PATH/$DTS_NAME.dts
-dtc -@ -Hepapr -I dts -O dtb -o /boot/overlays/$DTS_NAME.dtbo /tmp/$DTS_NAME
+dtc -@ -Hepapr -I dts -O dtb -o "$BOOT_PATH/overlays/$DTS_NAME.dtbo" /tmp/$DTS_NAME
 
 # Remove any configuration related to i2c and spi/spi1 and do the necessary changes for navigator
 echo "- Enable I2C, SPI and UART."
@@ -39,7 +52,7 @@ if ! grep -q "\[pi4\]" $CONFIG_FILE; then
 fi
 # find the line number of the [pi4] tag
 
-line_number=$(grep -n "\[pi4\]" $CONFIG_FILE | awk -F ":" '{print $1}')
+line_number=$(grep -n "\[pi4\]" $CONFIG_FILE | head -n 1 | awk -F ":" '{print $1}')
 echo "Line number of [pi4] tag: $line_number"
 
 

install/boards/bcm_28xx.sh

@@ -1,16 +1,30 @@
 #!/usr/bin/env bash
 
+set -e
+
 echo "Configuring BCM28XX board (Raspberry Pi zero, 1, 2, 3).."
 
-CMDLINE_FILE=/boot/cmdline.txt
+# Bookworm moved the boot partition to /boot/firmware and left plain text stubs behind at /boot,
+# so writing to /boot there configures nothing.
+if [ -f /boot/firmware/config.txt ]; then
+    BOOT_PATH=/boot/firmware
+elif [ -f /boot/config.txt ] && ! head -n 1 /boot/config.txt | grep -qx "DO NOT EDIT THIS FILE"; then
+    BOOT_PATH=/boot
+else
+    echo "No boot partition at /boot/firmware, and /boot is missing or holds the Bookworm stub." >&2
+    echo "Refusing to configure a file the board would never read." >&2
+    exit 1
+fi
+CMDLINE_FILE="$BOOT_PATH/cmdline.txt"
+CONFIG_FILE="$BOOT_PATH/config.txt"
 
 # Remove any configuration related to i2c and spi/spi1 and do the necessary changes for navigator
 echo "- Enable I2C, SPI and UART."
 for STRING in "dtparam=i2c_arm=" "dtparam=spi=" "dtoverlay=spi1" "dtoverlay=uart1"; do
-    sudo sed -i "/$STRING/d" /boot/config.txt
+    sudo sed -i "/$STRING/d" $CONFIG_FILE
 done
 for STRING in "dtparam=i2c_arm=on" "dtparam=spi=on" "dtoverlay=spi1-3cs" "dtoverlay=uart1"; do
-    echo "$STRING" | sudo tee -a /boot/config.txt
+    echo "$STRING" | sudo tee -a $CONFIG_FILE
 done
 
 # Check for valid modules file to load kernel modules