Skip to content

Say how many parts the guarded line has, and which one cannot be dropped - #135

Merged
openipc-ai merged 1 commit into
masterfrom
caveat-name-the-three-parts
Sep 7, 2026
Merged

Say how many parts the guarded line has, and which one cannot be dropped#135
openipc-ai merged 1 commit into
masterfrom
caveat-name-the-three-parts

Conversation

@openipc-ai

Copy link
Copy Markdown
Collaborator

The note that appears under a && block told readers to "enter the parts one at a time" without ever saying how many parts there are.

The reporter in OpenIPC/firmware#2381 counted two. Their XM U-Boot (2010.06-svn965) has no &&, so the guarded line came back as the tftp help entry — exactly the case this note exists for — and they re-entered the transfer and the sf write, dropping the sf erase that sat between them:

xmtech # tftp 0x82000000 openipc-hi3516cv300-nor-ultimate-16mb.bin && sf erase 0x0 0x1000000 && sf write 0x82000000 0x0 ${filesize}
tftp - tftp - download or upload image via network using TFTP protocol

xmtech # tftp 0x82000000 openipc-hi3516cv300-nor-ultimate-16mb.bin
Bytes transferred = 16777216 (1000000 hex)

xmtech # sf write 0x82000000 0x0 ${filesize}
Writing at 0x1000000 -- 100% complete.

NOR programming only clears bits, so that write stored old AND new across the whole 16MB starting at offset 0x0. It reported success and left a bootloader that cannot run — no serial output, no PHY link, and nothing in any log naming the step that was missed. It is the second camera lost at this line; #2299 is what put the note here in the first place.

The note now counts the three commands out by name and says the erase is the one that must not be left out. The sequential gate the previous wording established is unchanged and its test still pins it; the new test pins the count and the warning.

Quoting the locale values is not cosmetic — the new sentence puts a : inside the scalar, which YAML reads as a mapping. That took the whole en locale down until the suite caught it.

Translations updated for ru and zh alongside en.

Testing

Full suite in a Ruby 3.1.7 + MariaDB 11.8 container, matching the test job in build.yml:

360 runs, 1834 assertions, 0 failures, 0 errors, 0 skips

The note under a `&&` block told readers to "enter the parts one at a
time" without ever saying how many parts there are. The reporter in
OpenIPC/firmware#2381 counted two: their XM U-Boot answered the compound
line with the tftp help entry, and they re-entered the transfer and the
`sf write` but not the `sf erase` between them.

NOR programming only clears bits, so that write stored `old AND new`
across the whole 16MB starting at offset 0. It reported "Writing at
0x1000000 -- 100% complete." and left a bootloader that cannot run: no
serial output, no PHY link, and nothing in any log naming the step that
was missed. That is the second camera lost at this line -- #2299 is what
put the note here in the first place.

So the note now counts the three commands out by name and says the erase
is the one that must not be left out. The sequential gate the previous
wording established is unchanged, and its test still pins it.

Quoting the values is not cosmetic: the new sentence puts a `: ` inside
the scalar, which YAML reads as a mapping and which took the whole en
locale down until the suite caught it.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Clarify three-step guarded flash sequence and mandatory erase

🐞 Bug fix 📝 Documentation 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Names all three guarded flash commands and explicitly warns never to skip erase.
• Updates English, Russian, and Chinese safety guidance with YAML-safe quoting.
• Adds regression coverage for command count and the mandatory erase warning.
Diagram

graph TD
  A["Command list"] --> B{"Contains &&?"}
  B -- "No" --> C["Plain block"]
  B -- "Yes" --> D["Localized warning"] --> E["Transfer image"] --> F["Erase flash"] --> G["Write image"]
Loading
High-Level Assessment

The current approach is appropriate: it fixes the hazardous ambiguity directly where users encounter it, preserves the existing sequential-success rule, updates every supported translation, and adds focused regression coverage. Automatically splitting or executing compound commands was considered but would introduce bootloader-specific parsing risk and exceed the scope of this documentation safety fix.

Files changed (5) +24 / -3

Bug fix (1) +1 / -1
en.ymlMake the English flash warning explicit +1/-1

Make the English flash warning explicit

• Rewords the compound-command caveat to identify transfer, erase, and write as three required sequential commands. It explicitly warns that skipping erase can corrupt the bootloader and quotes the YAML scalar because the new text contains ': '.

config/locales/en.yml

Tests (1) +12 / -0
installation_helper_test.rbPin the command count and erase warning +12/-0

Pin the command count and erase warning

• Adds a regression test asserting that guarded flash guidance names transfer, erase, and write in order and explicitly says not to omit erase. Existing coverage continues to enforce sequential success gating and conditional warning display.

test/helpers/installation_helper_test.rb

Documentation (3) +11 / -2
installation_helper.rbDocument the omitted-erase failure scenario +9/-0

Document the omitted-erase failure scenario

• Expands the helper commentary with the real-world failure caused by interpreting the guarded line as two commands and skipping 'sf erase'. It explains why an unerased NOR write can report success while corrupting the bootloader.

app/helpers/installation_helper.rb

ru.ymlTranslate the expanded warning into Russian +1/-1

Translate the expanded warning into Russian

• Updates the Russian locale with the three-command sequence, sequential success requirement, and mandatory erase warning. The value is quoted to remain a valid YAML scalar.

config/locales/ru.yml

zh.ymlTranslate the expanded warning into Chinese +1/-1

Translate the expanded warning into Chinese

• Updates the Chinese locale to name all three flashing commands and explain the danger of writing without erasing. The value is quoted to prevent YAML from interpreting the embedded colon as a mapping.

config/locales/zh.yml

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@openipc-ai
openipc-ai merged commit 1d305ae into master Sep 7, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant