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

I2C slave: allow function wrapped callbacks #1835

Merged
merged 1 commit into from
Nov 30, 2022

Conversation

hitech95
Copy link
Contributor

@hitech95 hitech95 commented Sep 27, 2022

Summary

This PR fixes/implements the following bugs/features

This allow to have TwoWire callbacks methods inside classes. You can use std::bind to wrap the callback functions.

This is an example of a Class that completely handle the I2C comunication without having the user to manually implement the callbacks:

/*
 * Lib Functions
 */

I2Client::I2Client(TwoWire *bus)
{
    this->_bus = bus;
}

bool I2Client::begin(uint8_t baseAddress)
{
    _bus->begin(baseAddress);
    _bus->onRequest(std::bind(&I2Client::onHandleRequest, this));

    return true;
}

void I2Client::onHandleRequest()
{
    _bus->write("hello\n");
}

/*
 * Library Usage (main.cpp)
 */

#include <Arduino.h>
#include <Wire.h>

TwoWire bus(PB9, PB8);
I2Client client(&bus);

 void setup()
{
    client.begin(0x4E); // join i2c bus on address 0x4E
}

void loop()
{
    // put your main code here, to run repeatedly:
}

Validation

  • Ensure CI build is passed.
  • Demonstrate the code is solid. [e.g. Provide a sketch]

Code formatting

  • Ensure AStyle check is passed thanks CI

Closing issues

Closes #1833

@fpistm
Copy link
Member

fpistm commented Sep 28, 2022

Maybe related to #1617

@hitech95
Copy link
Contributor Author

@fpistm I was thinking the same, I've used the attachInterrupt as a reference.
If you think that the failover to void pointers is not necessary I can drop them.

Today I'll do more tests.

@fpistm
Copy link
Member

fpistm commented Sep 28, 2022

@fpistm I was thinking the same, I've used the attachInterrupt as a reference. If you think that the failover to void pointers is not necessary I can drop them.

Today I'll do more tests.

Yes it is probably the case but requires to be be tested 😉

@hitech95
Copy link
Contributor Author

hitech95 commented Sep 28, 2022

I've tested with a STM32G030C8T and a NUCLEO F401 and both works.
(One as a slave and the other as a master and the other way round)

@fpistm fpistm added this to In progress in STM32duino libraries via automation Sep 28, 2022
@fpistm fpistm removed this from In progress in STM32duino libraries Sep 28, 2022
@fpistm fpistm added this to In progress in STM32 core based on ST HAL via automation Sep 28, 2022
@fpistm
Copy link
Member

fpistm commented Oct 11, 2022

I've tested with a STM32G030C8T and a NUCLEO F401 and both works. (One as a slave and the other as a master and the other way round)

So you have tested with the c declaration removed ? if it works could you update the PR.

@hitech95
Copy link
Contributor Author

Not yet, I'm gonna do it tomorrow.
I've been quite busy with work.

@fpistm
Copy link
Member

fpistm commented Nov 18, 2022

Hi @hitech95
any update on this?

@hitech95
Copy link
Contributor Author

Hi
sorry I had lot of issues at work.
I'm gonna do some tests this weekend!

@fpistm
Copy link
Member

fpistm commented Nov 18, 2022

No worry. I understand perfectly 😉

@hitech95 hitech95 force-pushed the feature/wire-function-wrapper branch from 774cbac to 4aea869 Compare November 20, 2022 22:20
Signed-off-by: hitech95 <nicveronese@gmail.com>
@hitech95 hitech95 force-pushed the feature/wire-function-wrapper branch from 4aea869 to 8e1da5c Compare November 20, 2022 22:22
@hitech95
Copy link
Contributor Author

I've tested with the slave_receive and slave_sender examples from the basic arduino enviroment.
They works also by removing the C failbacks.

So I'm pushing a simplified patch for that.

Tested on Nucleo STM32 F401RE

@hitech95 hitech95 marked this pull request as ready for review November 20, 2022 22:24
Copy link
Member

@fpistm fpistm left a comment

Choose a reason for hiding this comment

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

LGTM.
Tested on several series.

Thank you @hitech95

STM32 core based on ST HAL automation moved this from In progress to Reviewer approved Nov 30, 2022
@fpistm fpistm added this to the 2.4.0 milestone Nov 30, 2022
@fpistm fpistm merged commit 0c2d58c into stm32duino:main Nov 30, 2022
STM32 core based on ST HAL automation moved this from Reviewer approved to Done Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Wire add support for callback_function_t callbacks
2 participants