Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added i2c buffer size to Wire interface #2292

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

perotom
Copy link

@perotom perotom commented Mar 3, 2021

Problem

If a custom buffer for i2c is used these functions will give libraries a better ability to determin the i2c buffer size.

Solution

Added two functions to return the assigned buffer size.

Example App

constexpr size_t I2C_BUFFER_SIZE = 128;

hal_i2c_config_t acquireWireBuffer() {
    hal_i2c_config_t config = {
        .size = sizeof(hal_i2c_config_t),
        .version = HAL_I2C_CONFIG_VERSION_1,
        .rx_buffer = new (std::nothrow) uint8_t[I2C_BUFFER_SIZE],
        .rx_buffer_size = I2C_BUFFER_SIZE,
        .tx_buffer = new (std::nothrow) uint8_t[I2C_BUFFER_SIZE],
        .tx_buffer_size = I2C_BUFFER_SIZE
    };
    return config;
}

void setup() {
   Serial.begin();
   Wire.begin();

   waitFor (Serial.isConnected, 10000);
   delay(1000);
   Serial.println(Wire.rxBufferSize());
}

void loop() {

}

Tested on B-Som platform.

References

Adds to #1112


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

If a custom buffer for i2c is used these functions will give libraries a better ability to determin the i2c buffer size.
@jlovinger-particle
Copy link

Without getting into the wider PR, lock usage in your hal_i2c_rx_buffer_size and hal_i2c_tx_buffer_size functions is unnecessary as a read with no modify is already atomic.

@perotom
Copy link
Author

perotom commented Mar 3, 2021

@jlovinger-particle Should I close the pull request and open another one?

@jlovinger-particle
Copy link

@jlovinger-particle Should I close the pull request and open another one?

I'm actually not on the engineering team so am not sure on the process. Just saw it pop up on my feed and had a quick comment.

@avtolstoy
Copy link
Member

@perotom You can just update your branch that is the source of this PR at any time while the PR is open.

I'll take a look at this later in more detail, but one suggestion is to potentially have a single HAL getter method that would fill in hal_i2c_config_t structure instead of discrete methods to just get the sizes of the buffers. Wiring API can be more specific for convenience and pick whatever is needed from hal_i2c_config_t.

Also removed unnecessary locks as a read with no modify is already atomic
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