steami: Fix READ_SECTOR command argument length and address.#1
steami: Fix READ_SECTOR command argument length and address.#1nedseb merged 4 commits intorelease_letssteamfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes the READ_SECTOR I2C command so it can correctly select and read the requested 256-byte sector/page from external flash, aligning the task handler with the I2C layer’s declared argument length.
Changes:
- Fix
TASK_READ_SECTORto accept 2 argument bytes and reconstruct a 16-bit sector number. - Fix
steami_flash_read_sector()to usesector_numberwhen computing the flash read address/length.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
source/board/steami/steami_flash.c |
Reads the correct flash address for the requested sector instead of always reading from 0. |
source/board/steami/steami32.c |
Updates READ_SECTOR task handling to accept 2 bytes and interpret them as a sector number. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Use STEAMI_FLASH_SECTOR constant instead of hardcoded 256 for TX data length - Use STEAMI_FLASH_FILE_ADDR base offset in steami_flash_read_sector for consistency
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 633cc4d697
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Limit sector_number bound check to STEAMI_FLASH_FILE_SIZE / STEAMI_FLASH_SECTOR (32752) instead of STEAMI_FLASH_NB_SECTOR (32768) to prevent reading into the reserved filename sector.
Summary
Three bugs in the STeaMi flash I2C bridge that affect data integrity:
Bug 1: READ_SECTOR argument length mismatch (
steami32.c)The I2C layer expects 2 argument bytes for READ_SECTOR, but the task handler checks
task_rx_len == 1. Sincetask_rx_lenis always 2, the command always fails.Fix: Check
task_rx_len == 2and reconstruct sector number from both bytes.Bug 2: READ_SECTOR hardcoded address (
steami_flash.c)steami_flash_read_sector()ignoressector_numberand always reads from address 0.Fix: Use
sector_number * STEAMI_FLASH_SECTORas the read offset.Bug 3: WRITE_DATA page boundary corruption (
steami32.c)In
TASK_WRITE_DATA_WRITE,bytes_wroteis a local variable reinitialized to 0 on each 30ms hook call. When a write crosses a 256-byte flash page boundary,append_filewrites only the remaining bytes in the current page. On the next hook iteration,bytes_wroteis 0 again, so the remaining data is written from the start of the buffer instead of the correct offset. This causes data corruption every ~256 bytes.Fix: Replace local
bytes_wrotewith a persistenttask_rx_offsetvariable, reset when a new WRITE_DATA command arrives.Impact: ~1 line in 12 is corrupted in a typical CSV write workload (22-byte lines, 256-byte pages).
Related
Test plan