Skip to content

fix: prevent UART FIFO overflow during I2C bulk write#207

Open
rahul31124 wants to merge 1 commit into
fossasia:mainfrom
rahul31124:i2c
Open

fix: prevent UART FIFO overflow during I2C bulk write#207
rahul31124 wants to merge 1 commit into
fossasia:mainfrom
rahul31124:i2c

Conversation

@rahul31124

@rahul31124 rahul31124 commented Jul 6, 2026

Copy link
Copy Markdown
Member

Fixes #206

Large UART transfers could overflow the 4-byte UART FIFO because the host sends data faster than the I2C bus can transmit it, causing dropped bytes and client timeouts.

Changes

  • Added a 255-byte RAM buffer to I2C_CommandWriteBulk.
  • Buffer the entire UART payload before starting the I2C transfer.
  • Prevent UART FIFO overflows and dropped bytes during large transfers.

Summary by Sourcery

Improve I2C bulk write handling by buffering incoming UART data and integrating watchdog servicing to ensure reliable large-transfer operation.

New Features:

  • Buffer UART bulk-write payload in RAM before starting I2C transmission to support larger transfers without data loss.

Bug Fixes:

  • Prevent UART FIFO overflow and dropped bytes during large I2C bulk writes by decoupling UART reception from I2C transmission.

Enhancements:

  • Add watchdog timer clears during UART buffering and I2C transmission to avoid unintended resets during long bulk writes.

@sourcery-ai

sourcery-ai Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Buffers UART payload into RAM before starting the I2C bulk write and adds watchdog servicing to prevent UART FIFO overflow and timeouts during large transfers.

Sequence diagram for buffered UART to I2C bulk write

sequenceDiagram
    actor Host
    participant UART1
    participant I2C_CommandWriteBulk
    participant I2C_bus
    participant Watchdog

    Host ->> UART1: send device, count, payload bytes
    UART1 ->> I2C_CommandWriteBulk: UART1_Read (device)
    UART1 ->> I2C_CommandWriteBulk: UART1_Read (count)

    loop count bytes
        UART1 ->> I2C_CommandWriteBulk: UART1_Read (payload byte)
        I2C_CommandWriteBulk ->> I2C_CommandWriteBulk: store in usb_buffer
        I2C_CommandWriteBulk ->> Watchdog: WATCHDOG_TimerClear
    end

    I2C_CommandWriteBulk ->> I2C_bus: I2C_StartSignal
    I2C_CommandWriteBulk ->> I2C_bus: I2C_Transmit (device << 1)

    loop count bytes
        I2C_CommandWriteBulk ->> I2C_bus: I2C_Transmit (usb_buffer[i])
        I2C_CommandWriteBulk ->> Watchdog: WATCHDOG_TimerClear
    end

    I2C_CommandWriteBulk ->> I2C_bus: I2C_StopSignal
Loading

File-Level Changes

Change Details Files
Buffer incoming UART data into a RAM array before performing the I2C bulk write, and service the watchdog during both UART reception and I2C transmission.
  • Introduce a static 255-byte buffer used by I2C_CommandWriteBulk to store the entire UART payload before I2C transmission.
  • Refactor I2C_CommandWriteBulk to first read device and count, then copy count bytes from UART into the buffer in a pre-transfer loop.
  • Update the transmission loop to send bytes from the RAM buffer over I2C instead of reading directly from UART.
  • Call the watchdog clear function regularly in both the UART read loop and the I2C transmit loop to avoid watchdog resets during long transfers.
src/bus/i2c/i2c.c

Assessment against linked issues

Issue Objective Addressed Explanation
#206 Decouple UART reception from I2C transmission in I2C_CommandWriteBulk by buffering the entire incoming payload before starting the I2C transfer (up to the 255-byte protocol limit).
#206 Prevent UART FIFO overflow and resulting data corruption/timeouts during large I2C bulk writes.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/bus/i2c/i2c.c
uint8_t device = UART1_Read();
uint8_t count = UART1_Read();

static uint8_t usb_buffer[255];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe payload[255]?

Comment thread src/bus/i2c/i2c.c

for (uint8_t i = 0; i < count; i++) {
usb_buffer[i] = UART1_Read();
WATCHDOG_TimerClear();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to clear the WDT regularly?

Comment thread src/bus/i2c/i2c.c
I2C_Transmit(UART1_Read());
for (uint8_t i = 0; i < count; i++) {
I2C_Transmit(usb_buffer[i]);
WATCHDOG_TimerClear();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here?

@CloudyPadmal CloudyPadmal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good with minor comments.

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.

I2C_CommandWriteBulk data corruption due to UART FIFO overflow during large transfers

2 participants