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.

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

Improvements in STM32 I2C native implementation #671

Merged
merged 2 commits into from
Apr 29, 2018
Merged

Improvements in STM32 I2C native implementation #671

merged 2 commits into from
Apr 29, 2018

Conversation

josesimoes
Copy link
Member

Description

  • I2C buffers are now static because of DMA requirements and are configurable at target level
  • Add I2C configuration files to all I2C enabled targets
  • I2C transactions are now performed on a working thread to allow CLR execution to move on as the I2C is doing it's thing in background using DMA
  • Update native declarations of I2C class lib
  • I2C NativeTransmit now returns an I2cTransferResult object
  • Update I2C NativeTransmit for ESP32
  • Add CLR event for I2C master
  • Rework CLR events declaration and flags to make it coherent and remove unused one
  • Add extra comment to CLR_RT_StackFrame::SetupTimeoutFromTicks in order to clarify the need to use a CLR_INT64 argument

Motivation and Context

How Has This Been Tested?

  • I2C app from Samples repo

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Signed-off-by: josesimoes [email protected]

@josesimoes josesimoes added Type: enhancement Area: Interpreter Everything related with the interpreter, execution engine and such Area: Targets labels Apr 26, 2018
@nfbot
Copy link
Member

nfbot commented Apr 26, 2018

Hi @josesimoes,

I'm nanoFramework bot.
Thank you for your contribution!

A human will be reviewing it shortly. 😉

@josesimoes josesimoes force-pushed the improvements-stm32-ic2 branch 8 times, most recently from af1b6ae to d90cc8d Compare April 26, 2018 15:19
AdrianSoundy
AdrianSoundy requested changes Apr 27, 2018
View reviewed changes
Copy link
Member

@AdrianSoundy AdrianSoundy left a comment

Choose a reason for hiding this comment

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

Review for ESP32 changes only. Left as synchronous returned result. Looks ok but won't be able to tell until tested. Extra work required to make asynchronous. Will look to doing this once this change is committed.
Needs the STM32 specific defines removed from DevKitC/nanoCLR/Windows.Devices.I2c/win_dev_i2c_native_Windows_Devices_I2C_I2cDevice.cpp at line 46

@josesimoes
Improvements in STM32 I2C native implementation
89b72e9 
- I2C buffers are now static because of DMA requirements and are configurable at target level
- Add I2C configuration files to all I2C enabled targets
- I2C transactions are now performed on a working thread to allow CLR execution to move on as the I2C is doing it's thing in background using DMA
- Update native declarations of I2C class lib
- I2C NativeTransmit now returns an I2cTransferResult object
- Update I2C NativeTransmit for ESP32
- Add CLR event for I2C master
- Rework CLR events declaration and flags to make it coherent and remove unused one
- Add extra comment to CLR_RT_StackFrame::SetupTimeoutFromTicks in order to clarify the need to use a CLR_INT64 argument

Signed-off-by: josesimoes <[email protected]>
@josesimoes
Copy link
Member Author

@AdrianSoundy I've just remove the STM32 specific stuff from ESP32 code.

For small buffer sizes, there is no big deal with keeping the implementation blocking the CLR. The thing gets complicated when the buffer increases...

Does the ESP32 offers an "async" API for the peripherals such I2C? Is it using DMA internally? (just being curious 😃 )

@AdrianSoundy
Copy link
Member

AdrianSoundy commented Apr 27, 2018 •
edited
Loading

The Esp32 has a 32 byte memory mapped buffer which it fills from an ISR. for each I2C segment.
The main command that starts the transfer is synchronous so to make it async. I would need to put into separate thread. Will leave it as it is for now.

@AdrianSoundy AdrianSoundy merged commit 0aa43c7 into nanoframework:develop Apr 29, 2018
@josesimoes josesimoes deleted the improvements-stm32-ic2 branch April 30, 2018 07:41
@josesimoes josesimoes mentioned this pull request Apr 30, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AdrianSoundy AdrianSoundy AdrianSoundy approved these changes

@piwi1263 piwi1263 Awaiting requested review from piwi1263

Assignees
No one assigned
Labels
Area: Interpreter Everything related with the interpreter, execution engine and such Area: Targets Type: enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Applications using I2C become irresponsive after random execution time.
3 participants
@josesimoes @nfbot @AdrianSoundy