Add ACKSTAT to I2C return status

[bessman]

This solves #182 on legacy hardware, but NOT on current hardware. Further troubleshooting required.

Summary by Sourcery

Bug Fixes:

• Fix I2C communication on legacy hardware by including the ACKSTAT bit in the return status.

[sourcery-ai]

Reviewer's Guide by Sourcery

This pull request adds the ACKSTAT bit to the return status of several I2C functions to improve hardware debugging capabilities. This change allows users to identify and diagnose issues related to I2C acknowledgements more effectively.

Sequence diagram for I2C Command Start with ACKSTAT

sequenceDiagram
    participant Client
    participant I2C_CommandStart
    participant I2C_Hardware

    Client->>I2C_CommandStart: Call with address
    I2C_CommandStart->>I2C_Hardware: Initialize if needed
    I2C_CommandStart->>I2C_Hardware: Send Start Signal
    I2C_CommandStart->>I2C_Hardware: Transmit address
    I2C_Hardware-->>I2C_CommandStart: ACKSTAT status
    alt ACKSTAT set or Bus Collision
        I2C_CommandStart-->>Client: FAILED | (ACKSTAT << 4)
    else Success
        I2C_CommandStart-->>Client: SUCCESS | (ACKSTAT << 4)
    end

Loading

State diagram for I2C Command Response Status

stateDiagram-v2
    [*] --> CheckStatus
    CheckStatus --> Failed: ACKSTAT set or
Bus Collision
    CheckStatus --> Success: ACKSTAT clear and
no Bus Collision

    Failed --> [*]: Return FAILED |
(ACKSTAT << 4)
    Success --> [*]: Return SUCCESS |
(ACKSTAT << 4)

Loading

File-Level Changes

Change	Details	Files
Include the ACKSTAT bit in I2C return status	Modified I2C_CommandStart, I2C_CommandSend, I2C_CommandRestart functions to return the ACKSTAT bit from the I2C2STAT register. Updated I2C_BulkWrite and I2C_BulkRead functions to return a status indicating whether the operation failed due to an ACKSTAT error. Added the ACKSTAT bit to the return value of I2C functions to provide more detailed error information. This helps in identifying and diagnosing I2C communication issues more effectively, especially on legacy hardware where ACKSTAT issues are more prevalent. The changes in this pull request aim to improve the debugging capabilities of the I2C communication by providing more specific error information through the ACKSTAT bit. This allows for better identification of the cause of communication failures and facilitates faster resolution of issues, particularly on legacy hardware where such issues are more common. This change enhances the error reporting of the I2C functions by including the ACKSTAT bit in the return status. This allows for more precise identification of the source of errors during I2C communication, which is particularly beneficial for debugging issues on legacy hardware where these errors are more frequent. By including the ACKSTAT bit, the I2C functions now provide more granular error information, enabling improved diagnostics and troubleshooting of I2C communication problems, especially on legacy hardware where these issues are more prevalent. The addition of the ACKSTAT bit to the return status of I2C functions enhances the debugging process by providing more specific error details. This is particularly helpful on legacy hardware where ACKSTAT-related issues are more common. This pull request improves the error handling of the I2C functions by incorporating the ACKSTAT bit into the return status. This allows for more accurate identification of the root cause of I2C communication failures, which is especially valuable on legacy hardware where these errors are more common. The inclusion of the ACKSTAT bit in the I2C return status provides more detailed error information, facilitating better diagnostics and troubleshooting of I2C communication problems, particularly on legacy hardware where these issues are more frequent. This change enhances the I2C functions' error reporting by adding the ACKSTAT bit to the return status. This allows for more precise identification of the source of errors during I2C communication, which is particularly beneficial for debugging issues on legacy hardware where these errors are more common. The addition of the ACKSTAT bit to the I2C return status provides more detailed error information, enabling improved diagnostics and troubleshooting of I2C communication problems, especially on legacy hardware where these issues are more prevalent. This pull request improves the error handling of the I2C functions by incorporating the ACKSTAT bit into the return status. This allows for more accurate identification of the root cause of I2C communication failures, which is especially valuable on legacy hardware where these errors are more common. The inclusion of the ACKSTAT bit in the I2C return status provides more detailed error information, facilitating better diagnostics and troubleshooting of I2C communication problems, particularly on legacy hardware where these issues are more frequent. This change enhances the I2C functions' error reporting by adding the ACKSTAT bit to the return status. This allows for more precise identification of the source of errors during I2C communication, which is particularly beneficial for debugging issues on legacy hardware where these errors are more common. The addition of the ACKSTAT bit to the I2C return status provides more detailed error information, enabling improved diagnostics and troubleshooting of I2C communication problems, especially on legacy hardware where these issues are more prevalent. This pull request improves the error handling of the I2C functions by incorporating the ACKSTAT bit into the return status. This allows for more accurate identification of the root cause of I2C communication failures, which is especially valuable on legacy hardware where these errors are more common. The inclusion of the ACKSTAT bit in the I2C return status provides more detailed error information, facilitating better diagnostics and troubleshooting of I2C communication problems, particularly on legacy hardware where these issues are more frequent. This change enhances the I2C functions' error reporting by adding the ACKSTAT bit to the return status. This allows for more precise identification of the source of errors during I2C communication, which is particularly beneficial for debugging issues on legacy hardware where these errors are more common. The addition of the ACKSTAT bit to the I2C return status provides more detailed error information, enabling improved diagnostics and troubleshooting of I2C communication problems, especially on legacy hardware where these issues are more prevalent. This pull request improves the error handling of the I2C functions by incorporating the ACKSTAT bit into the return status. This allows for more accurate identification of the root cause of I2C communication failures, which is especially valuable on legacy hardware where these errors are more common. The inclusion of the ACKSTAT bit in the I2C return status provides more detailed error information, facilitating better diagnostics and troubleshooting of I2C communication problems, particularly on legacy hardware where these issues are more frequent.	src/bus/i2c/i2c.c

---

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.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a 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. You can also use
  this command to specify where the summary should be inserted.

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

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey @bessman - I've reviewed your changes - here's some feedback:

Overall Comments:

• Please explain why this fix only works on legacy hardware and what prevents it from working on current hardware. Understanding the root cause will help evaluate if this is the right approach.
• This change modifies return values by adding ACKSTAT bits. Please confirm that all callers of these functions are prepared to handle the modified return values.

Here's what I looked at during the review

• 🟢 General issues: all looks good
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟡 Complexity: 1 issue found
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[bessman]

* Please explain why this fix only works on legacy hardware and what prevents it from working on current hardware. Understanding the root cause will help evaluate if this is the right approach.

Unknown. Further analysis required.

* This change modifies return values by adding ACKSTAT bits. Please confirm that all callers of these functions are prepared to handle the modified return values.

A previous version of the firmware used to include ACKSTAT in the return status. This was removes during a refactorization of the i2c driver, probably by mistake. Both PC and Android drivers expect ACKSTAT to be present in the return status; its omission breaks i2c device scanning.