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

Improvements in STM32 I2C class lib code #685

Merged
merged 7 commits into from
May 24, 2018

Conversation

josesimoes
Copy link
Member

@josesimoes josesimoes commented May 14, 2018

Description

  • Remove static write and read buffers for I2C
  • Add function to compute estimated time to perform I2C transaction
  • Rework I2C NativeTransmit to use CLR events (or not) depending on transaction time
  • Remove I2C init and uninit calls (not required anymore)
  • Remove target configurations for I2C (not required anymore)

Motivation and Context

How Has This Been Tested?

All tests reading and writing from/to STMPE811 touch controller in STM32F429I-DISCO.
The writings were performed to setup the touchscreen controller operation which was returning coordinates as expected.

Reading ChipID inside loop (straight through operation, no CLR thread swap)

            byte[] buffer = new byte[2];
            Windows.Devices.I2c.I2cTransferResult readResult;

            for (; ; )
            {
                readResult = _touchController.WriteReadPartial(new byte[] { 0 }, buffer);
            }

image
image

Time between transactions: 0.228[ms]

Reading 220 bytes from chip inside loop (exceeds thread execution quantum, continuation setup, returning transaction result on following execution)

            byte[] buffer = new byte[220];
            Windows.Devices.I2c.I2cTransferResult readResult;

            for (; ; )
            {
                readResult = _touchController.WriteReadPartial(new byte[] { 0 }, buffer);
            }

image

Time to setup read address and read 220 bytes : ~18[ms]
Time between transactions: 10.81[ms]

Reading ChipID on 10 consecutive transactions (straight through operation, no CLR thread swap)

            byte[] buffer = new byte[2];
            Windows.Devices.I2c.I2cTransferResult readResult;

            for (; ; )
            {
                readResult = _touchController.WriteReadPartial(new byte[] { 0 }, buffer);
                readResult = _touchController.WriteReadPartial(new byte[] { 0 }, buffer);
                readResult = _touchController.WriteReadPartial(new byte[] { 0 }, buffer);
                readResult = _touchController.WriteReadPartial(new byte[] { 0 }, buffer);
                readResult = _touchController.WriteReadPartial(new byte[] { 0 }, buffer);
                readResult = _touchController.WriteReadPartial(new byte[] { 0 }, buffer);
                readResult = _touchController.WriteReadPartial(new byte[] { 0 }, buffer);
                readResult = _touchController.WriteReadPartial(new byte[] { 0 }, buffer);
                readResult = _touchController.WriteReadPartial(new byte[] { 0 }, buffer);
                readResult = _touchController.WriteReadPartial(new byte[] { 0 }, buffer);
            }

image

Time to setup read address and read 2 bytes : ~0.47[ms]
Time between transactions: 0.1924[ms]

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 jose.simoes@eclo.solutions

@josesimoes josesimoes added Type: enhancement Series: STM32xx Everything related specifically with STM32 targets labels May 14, 2018
@josesimoes josesimoes requested a review from a user May 14, 2018 16:12
@nfbot
Copy link
Member

nfbot commented May 14, 2018

Hi @josesimoes,

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

A human will be reviewing it shortly. 😉

- Remove static write and read buffers for I2C
- Add function to compute estimated time to perform I2C transaction
- Rework I2C NativeTransmit to use CLR events (or not) depending on transaction time
- Remove I2C init and uninit calls (not required anymore)
- Remove target configurations for I2C (not required anymore)

Signed-off-by: josesimoes <jose.simoes@eclo.solutions>
@josesimoes josesimoes force-pushed the improvements-i2c-stm32 branch from 7a77074 to bdd08b9 Compare May 14, 2018 16:19
@ghost
Copy link

ghost commented May 14, 2018

There must be something wrong as nothing has changed on my side :(
I still have the 10ms penalty and the duplicate data when I read :

image

image

@josesimoes
Copy link
Member Author

You seem to have the correct version.

That timing you see there between transactions is because of that Thread.Sleep()
When you call that, the current thread immediately stops execution and reschedule.
All other threads in the queue - and eligible to run - will get their execution share and the engine gets back to this one which, in the mean time, has become eligible again.

For such short timing I would recommend either do nothing or do a couple of dummy operations just to respect those 5ms that I guess are required for the write operation to complete.
Anyway you can check with logical analyser if the correct timing is happening.

@josesimoes josesimoes requested review from piwi1263 and removed request for a user May 16, 2018 07:39
Copy link
Member

@piwi1263 piwi1263 left a comment

Choose a reason for hiding this comment

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

LGTM

@josesimoes josesimoes merged commit 1d31c91 into nanoframework:develop May 24, 2018
@josesimoes josesimoes deleted the improvements-i2c-stm32 branch May 24, 2018 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Series: STM32xx Everything related specifically with STM32 targets Type: enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

I2C does not send correct data
3 participants