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

Fix i2c communication with pcf8574 on stm32 targets of f1, f2, f4 and l1 families #4365

Merged
merged 2 commits into from
Jun 5, 2017

Conversation

LMESTM
Copy link
Contributor

@LMESTM LMESTM commented May 22, 2017

Description

This PR fixes issue reported in #4214
This requires an update in HAL but also a fix in the GPIO management of F1 family targets

Status

READY

Tests

Test reported in #4214 is passed ok with the fix.
CI test cases passed ok as well on all 4 families where fix applies:

+-------------------+---------------+---------------+--------+--------------------+-------------+
| target | platform_name | test suite | result | elapsed_time (sec) | copy_method |
+-------------------+---------------+---------------+--------+--------------------+-------------+
| NUCLEO_F091RC-ARM | NUCLEO_F091RC | tests-api-i2c | OK | 59.24 | shell |
| NUCLEO_F103RB-ARM | NUCLEO_F103RB | tests-api-i2c | OK | 73.24 | shell |
| NUCLEO_F207ZG-ARM | NUCLEO_F207ZG | tests-api-i2c | OK | 58.66 | shell |
| NUCLEO_F303ZE-ARM | NUCLEO_F303ZE | tests-api-i2c | OK | 61.36 | shell |
| NUCLEO_F446RE-ARM | NUCLEO_F446RE | tests-api-i2c | OK | 59.19 | shell |
| NUCLEO_F767ZI-ARM | NUCLEO_F767ZI | tests-api-i2c | OK | 59.74 | shell |
| NUCLEO_L073RZ-ARM | NUCLEO_L073RZ | tests-api-i2c | OK | 63.78 | shell |
| NUCLEO_L152RE-ARM | NUCLEO_L152RE | tests-api-i2c | OK | 59.62 | shell |
| NUCLEO_L476RG-ARM | NUCLEO_L476RG | tests-api-i2c | OK | 58.96 | shell |
+-------------------+---------------+---------------+--------+--------------------+-------------+

@theotherjimmy
Copy link
Contributor

@LMESTM I'm not sure that I know what "c pcf f1 f2 f4 l1" is supposed to mean. Could you update the title?

@LMESTM LMESTM changed the title Fix i2 c pcf f1 f2 f4 l1 Fix i2c communication with pcf8574 on stm32 targets of f1, f2, f4 and l1 families May 22, 2017
@LMESTM
Copy link
Contributor Author

LMESTM commented May 22, 2017

@theotherjimmy - sure, hope it's better now

@theotherjimmy
Copy link
Contributor

@LMESTM Much better! I now understand what this PR does!

@0xc0170
Copy link
Contributor

0xc0170 commented May 25, 2017

/morph test

@mbed-bot
Copy link

Result: ABORTED

Your command has finished executing! Here's what you wrote!

/morph test

@bridadan
Copy link
Contributor

Sorry for the ABORT everyone, need to prioritize a few PR CI jobs for the upcoming release.

@adbridge
Copy link
Contributor

@LMESTM Looks like this needs a rebase please.

@sg-
Copy link
Contributor

sg- commented May 30, 2017

@LMESTM can you resolve the conflict?

@LMESTM
Copy link
Contributor Author

LMESTM commented May 31, 2017

@sg- @adbridge Sure - rebase done

There were still side effects, in particular on I2C master slave test,
when setting by default the Pin Speed for F1 family. So for F1 family,
let's do it only in case of Output which is the only case where this
actually applies on this family.
As reported in issue ARMmbed#4214, there are seen issues seen first on
NUCLEO_F103RB in case of successive Reads of 1 byte at a time.

This issue is due to a wrong state management in the end of read sequence.
Also F1 i2c driver was not fully aligned to others, which is updated here.
@LMESTM
Copy link
Contributor Author

LMESTM commented Jun 2, 2017

rebased again ...

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 5, 2017

cc @Willem23

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 5, 2017

/morph test

@mbed-bot
Copy link

mbed-bot commented Jun 5, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 442

All builds and test passed!

@adbridge
Copy link
Contributor

adbridge commented Jun 5, 2017

Some of the changes are dependent on #4402 which contains API changes and was thus targeted at 5.5.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants