Skip to content

I2C slave: allow function wrapped callbacks #1835

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 30, 2022
Merged

I2C slave: allow function wrapped callbacks #1835

merged 1 commit into from
Nov 30, 2022

Conversation

hitech95
Copy link
Contributor

@hitech95 hitech95 commented Sep 27, 2022 •
edited
Loading

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

@hitech95 hitech95 force-pushed the feature/wire-function-wrapper branch from c528a21 to 774cbac Compare September 27, 2022 22:12
@fpistm fpistm requested a review from ABOSTM September 28, 2022 08:24
@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 •
edited
Loading

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 the enhancement New feature or request label 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
@hitech95
I2C slave: allow function wrapped callbacks
8e1da5c 
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
fpistm
fpistm approved these changes Nov 30, 2022
View reviewed changes
Copy link
Member

@fpistm fpistm left a comment •
edited
Loading

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

@fpistm fpistm added this to the 2.4.0 milestone Nov 30, 2022
@fpistm fpistm merged commit 0c2d58c into stm32duino:main Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fpistm fpistm fpistm approved these changes

@ABOSTM ABOSTM Awaiting requested review from ABOSTM

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
2.4.0
Development

Successfully merging this pull request may close these issues.

Wire add support for callback_function_t callbacks
2 participants
@hitech95 @fpistm