Skip to content

Conversation

@leon0399
Copy link
Member

No description provided.

@leon0399 leon0399 force-pushed the feature/c-api branch 8 times, most recently from 6467c95 to f51c4f3 Compare April 16, 2025 13:31
@leon0399 leon0399 requested a review from Copilot April 16, 2025 13:36

This comment was marked as outdated.

@leon0399 leon0399 force-pushed the feature/c-api branch 6 times, most recently from da47402 to 201a4e2 Compare April 20, 2025 10:58
@leon0399 leon0399 requested a review from Copilot April 20, 2025 10:59

This comment was marked as outdated.

@leon0399 leon0399 force-pushed the feature/c-api branch 10 times, most recently from ebf7bd4 to e5c373d Compare April 21, 2025 21:25
@leon0399 leon0399 self-assigned this Apr 21, 2025
@leon0399 leon0399 force-pushed the feature/c-api branch 6 times, most recently from 75d4add to 2f8497f Compare April 25, 2025 21:39
@leon0399 leon0399 requested a review from Copilot April 25, 2025 22:16
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a pure C API for Arduino-based I²C device interactions while refactoring several device implementations (e.g. PCA9685 and MAX170XX) to use the new API. Key changes include:

  • Addition of src/i2cdevbus_hal_arduino.hpp to provide the pure C API.
  • Refactoring of device drivers in pca9685 and max170xx to use the new API calls.
  • Updates to examples to include the new header and improve error handling.

Reviewed Changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/i2cdevbus_hal_arduino.hpp New pure C API implementation for Arduino hardware abstraction.
src/i2cdev/pca9685.hpp & pca9685.h Refactored PCA9685 driver to use the new API and updated macros.
src/i2cdev/max170xx.hpp & max170xx.h Updated MAX170XX driver implementations to rely on the C API.
src/i2cdev/common.hpp New header adding a Result template for improved error handling.
hal/I2CDevBus_ArduinoWire.hpp & I2CDevLib.h Removed obsolete ArduinoWire based implementation.
examples/* Examples updated to include the new header and add proper casting and error checks.
Files not reviewed (1)
  • platformio.ini: Language not supported

@leon0399 leon0399 changed the base branch from master to develop April 26, 2025 16:11
@leon0399 leon0399 marked this pull request as ready for review April 26, 2025 16:18
@leon0399
Copy link
Member Author

@CodiumAI-Agent /review

@QodoAI-Agent
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 5 🔵🔵🔵🔵🔵
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Incorrect Member

PCA9685 constructor assigns to i2cdev instead of the inherited bus member, likely causing runtime errors.

{
    this->addr = addr;
    this->i2cdev = &bus;
}
Uninitialized Prescale

The _prescale field is uninitialized before use in writeMicroseconds, leading to incorrect tick calculations.

        uint32_t _oscillator_freq = PCA9685_OSCILLATOR_FREQ;
        uint8_t _prescale;

    public:
#ifdef I2CDEVLIB_DEFAULT_BUS
        PCA9685(uint8_t addr = PCA9685_I2CADDR_BASE, i2cdev_bus_t& bus = I2CDEVLIB_DEFAULT_BUS)
#else
        PCA9685(uint8_t addr, i2cdev_bus_t& bus)
#endif
        {
            this->addr = addr;
            this->i2cdev = &bus;
        }

        void setOscillatorFrequency(uint32_t freq) { this->_oscillator_freq = freq; }

@leon0399
Copy link
Member Author

@CodiumAI-Agent /describe

@QodoAI-Agent
Copy link

QodoAI-Agent commented Apr 26, 2025

Title

(Describe updated until commit f937efe)

✨ (API): add pure C API


PR Type

Enhancement


Description

  • Introduce pure-C I2C API for devices

  • Refactor C++ wrappers to call C API

  • Add Arduino HAL and Result helper

  • Update examples and add board configs


Changes walkthrough 📝

Relevant files
Enhancement
11 files
mpu6050.hpp
Refactor MPU6050 C++ wrapper to C API                                       
+217/-284
pca9685.hpp
Refactor PCA9685 C++ wrapper to C API                                       
+57/-154
max170xx.hpp
Refactor MAX170xx C++ wrapper to C API                                     
+66/-91 
i2cdevbus_hal_arduino.hpp
Add Arduino I2C bus HAL implementation                                     
+205/-0 
common.hpp
Add Result wrapper type                                                       
+18/-0   
mpu6050.h
Expand MPU6050 C API and enums                                                     
+871/-114
pca9685.h
Expand PCA9685 C API definitions                                                 
+198/-12
BasicReadings.ino
Update MPU6050 example to new API                                               
+2/-3     
Simple.ino
Revise MAX170XX example for Result API                                     
+11/-3   
VibroPulse.ino
Fix VibroPulse example API calls                                                 
+3/-3     
Servo.ino
Update Servo example includes and calls                                   
+1/-1     
Configuration changes
1 files
platformio.ini
Add new board environments configuration                                 
+10/-0   
Additional files
5 files
I2CDevLib.h +0/-19   
I2CDevBus_ArduinoWire.hpp +0/-169 
max170xx.h +171/-6 
i2cdevbus.h +525/-26
i2cdevbus.hpp +0/-506 

Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link

    Copilot AI left a comment

    Choose a reason for hiding this comment

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

    Pull Request Overview

    This pull request introduces a pure C API for the I2C device communication library while refactoring the existing C++ drivers and examples to use the new API.

    • Added a new header (i2cdevbus_hal_arduino.hpp) that implements pure C API functionality for Arduino.
    • Updated sensor driver headers (pca9685, max170xx, common.hpp) to use inline pure C functions and modify class implementations accordingly.
    • Removed deprecated headers (I2CDevLib.h and I2CDevBus_ArduinoWire.hpp) and updated examples to reflect the new API.

    Reviewed Changes

    Copilot reviewed 16 out of 17 changed files in this pull request and generated no comments.

    Show a summary per file
    File Description
    src/i2cdevbus_hal_arduino.hpp New header providing C API functions for Arduino.
    src/i2cdev/pca9685.hpp Updated PCA9685 driver to call pure C API functions.
    src/i2cdev/pca9685.h Refactored inline C implementations using the new pure C API methods.
    src/i2cdev/max170xx.hpp Updated MAX170XX driver classes to use the new API with inline C function calls.
    src/i2cdev/max170xx.h Introduced new inline functions that implement the pure C API for MAX170XX devices.
    src/i2cdev/common.hpp New header providing a simple Result template for improved error handling.
    src/hal/I2CDevBus_ArduinoWire.hpp Removed legacy C++ I2C bus implementation for Arduino.
    src/I2CDevLib.h Removed deprecated legacy header.
    examples/* Updated examples to use the new header and API calls.
    Files not reviewed (1)
    • platformio.ini: Language not supported
    Comments suppressed due to low confidence (1)

    examples/MAX170XX/Simple/Simple.ino:4

    • The example references 'MAX17048', but no matching class is defined in the drivers. Consider using one of the defined classes (e.g., MAX17043, MAX17044, MAX170x8, or MAX170x9) or add an alias for MAX17048 if intended.
    i2cdev::MAX17048 max17048;
    

    @leon0399 leon0399 merged commit 1c87d42 into develop Apr 26, 2025
    34 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.

    3 participants