SPI::write: fix number of transmitted symbols. #9624

mprse · 2019-02-06T08:15:56Z

Description

This is a fix to issue reported by @jeromecoutant : spi communication problem with ism43362 module on spi feature branch.

It looks like there is a bug in SPI::write. Independently on symbol size number of transferred symbols should be always 1. This PR should fix this problem.

mbed test -t GCC_ARM -m DISCO_F413ZH -n tests-netsocket-dns -v

| target               | platform_name | test suite          | result | elapsed_time (sec) | copy_method |
|----------------------|---------------|---------------------|--------|--------------------|-------------|
| DISCO_F413ZH-GCC_ARM | DISCO_F413ZH  | tests-netsocket-dns | OK     | 140.31             | default     |
mbedgt: test suite results: 1 OK
mbedgt: test case report:
| target               | platform_name | test suite          | test case                             | passed | failed | result | elapsed_time (sec) |
|----------------------|---------------|---------------------|---------------------------------------|--------|--------|--------|--------------------|
| DISCO_F413ZH-GCC_ARM | DISCO_F413ZH  | tests-netsocket-dns | ASYNCHRONOUS_DNS                      | 1      | 0      | OK     | 0.27               |
| DISCO_F413ZH-GCC_ARM | DISCO_F413ZH  | tests-netsocket-dns | ASYNCHRONOUS_DNS_CACHE                | 1      | 0      | OK     | 0.69               |
| DISCO_F413ZH-GCC_ARM | DISCO_F413ZH  | tests-netsocket-dns | ASYNCHRONOUS_DNS_CANCEL               | 1      | 0      | OK     | 5.35               |
| DISCO_F413ZH-GCC_ARM | DISCO_F413ZH  | tests-netsocket-dns | ASYNCHRONOUS_DNS_EXTERNAL_EVENT_QUEUE | 1      | 0      | OK     | 3.49               |
| DISCO_F413ZH-GCC_ARM | DISCO_F413ZH  | tests-netsocket-dns | ASYNCHRONOUS_DNS_INVALID_HOST         | 1      | 0      | OK     | 1.66               |
| DISCO_F413ZH-GCC_ARM | DISCO_F413ZH  | tests-netsocket-dns | ASYNCHRONOUS_DNS_NON_ASYNC_AND_ASYNC  | 1      | 0      | OK     | 0.79               |
| DISCO_F413ZH-GCC_ARM | DISCO_F413ZH  | tests-netsocket-dns | ASYNCHRONOUS_DNS_SIMULTANEOUS         | 1      | 0      | OK     | 1.2                |
| DISCO_F413ZH-GCC_ARM | DISCO_F413ZH  | tests-netsocket-dns | ASYNCHRONOUS_DNS_SIMULTANEOUS_CACHE   | 1      | 0      | OK     | 0.86               |
| DISCO_F413ZH-GCC_ARM | DISCO_F413ZH  | tests-netsocket-dns | ASYNCHRONOUS_DNS_SIMULTANEOUS_REPEAT  | 1      | 0      | OK     | 65.63              |
| DISCO_F413ZH-GCC_ARM | DISCO_F413ZH  | tests-netsocket-dns | ASYNCHRONOUS_DNS_TIMEOUTS             | 1      | 0      | OK     | 3.04               |
| DISCO_F413ZH-GCC_ARM | DISCO_F413ZH  | tests-netsocket-dns | SYNCHRONOUS_DNS                       | 1      | 0      | OK     | 0.12               |
| DISCO_F413ZH-GCC_ARM | DISCO_F413ZH  | tests-netsocket-dns | SYNCHRONOUS_DNS_INVALID               | 1      | 0      | OK     | 31.99              |
| DISCO_F413ZH-GCC_ARM | DISCO_F413ZH  | tests-netsocket-dns | SYNCHRONOUS_DNS_MULTIPLE              | 1      | 0      | OK     | 0.5                |
mbedgt: test case results: 13 OK

Please note that to run the test spi support for DISCO_F413ZH needs to be enabled.

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@jeromecoutant @ithinuel

jeromecoutant · 2019-02-06T09:05:47Z

Independently on symbol size number of transferred symbols should be always 1

Maybe API should be updated to remove this "fixed" (not useful) parameter ?

mprse · 2019-02-06T09:50:21Z

Maybe API should be updated to remove this "fixed" (not useful) parameter ?

This parameters are required by the new SPI HAL API:
https://github.com/ARMmbed/mbed-os/blob/feature-hal-spec-spi/docs/design-documents/hal/0000-spi-overhaul.md

ciarmcom · 2019-02-06T10:00:29Z

@mprse, thank you for your changes.
@ithinuel @ARMmbed/mbed-os-maintainers please review.

adbridge · 2019-02-06T10:27:22Z

@mprse Looks like you have some formatting issues, can you please take a look at the astyle failures.

mprse · 2019-02-06T10:35:09Z

@mprse Looks like you have some formatting issues, can you please take a look at the astyle failures.

Style issue has been fixed.

0xc0170 · 2019-02-11T11:20:12Z

@ithinuel Final approval?

Ci triggered meantime

ithinuel

LGTM

mbed-ci · 2019-02-11T13:59:18Z

Test run: FAILED

Summary: 3 of 8 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-IAR
jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM

0xc0170 · 2019-02-11T14:02:04Z

[Error] BufferedSpi.h@71,0: #262: not a class or struct name from the build failures, please review

mprse · 2019-02-14T08:48:55Z

The build error is very odd. We got mbed-os build error for DISCO_L475VG_IOT01A board only. Build error is caused by the wifi-ism43362 driver which uses SPI and SPI is disabled for this board on the feature branch.

d -o BUILD/tests/DISCO_L475VG_IOT01A/GCC_ARM/wifi-ism43362/ISM43362/ATParser/ATParser.o ./wifi-ism43362/ISM43362/ATParser/ATParser.cpp
[Error] BufferedSpi.h@71,32: expected class-name before '{' token

@0xc0170 @cmonr Why wifi-ism43362 driver is included in the CI build for DISCO_L475VG_IOT01A ? I checked that it is not in case DISCO_F413ZH.

0xc0170 · 2019-02-14T10:55:05Z

@0xc0170 @cmonr Why wifi-ism43362 driver is included in the CI build for DISCO_L475VG_IOT01A ? I checked that it is not in case DISCO_F413ZH.

I don't see anything related set in targets.json file, could be set in CI config? @ARMmbed/mbed-os-test Please review

cmonr · 2019-02-15T22:49:36Z

@mprse Could this line be the source of problems? https://github.com/ARMmbed/mbed-os/blob/feature-hal-spec-spi/targets/targets.json#L3748

When the feature branch was last rebased, was it tested against CI?

mprse · 2019-02-19T10:45:59Z

@mprse Could this line be the source of problems? https://github.com/ARMmbed/mbed-os/blob/feature-hal-spec-spi/targets/targets.json#L3748

I don't think so. The branch was recently rebased and CI has passed here last time:
PR #7976
Above PR was the last modification on the feature branch I think:
https://github.com/ARMmbed/mbed-os/commits/feature-hal-spec-spi

I have now idea what is causing this CI failures 😢

0xc0170 · 2019-02-19T10:57:18Z

As it was few days back, I restarted CI and lets review it to resolve the failure if any

mbed-ci · 2019-02-19T14:03:06Z

Test run: FAILED

Summary: 3 of 8 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-IAR

mprse · 2019-04-15T12:23:52Z

@0xc0170 This one is ready.

mbed-ci · 2019-04-17T01:18:57Z

Test run: FAILED

Summary: 1 of 7 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM

mprse · 2019-04-17T06:28:42Z

Test run: FAILED

The error looks not related (TB_SENSE_1:ARM only):

usage: examples.py [-h] [-c CONFIG] [-e EXAMPLE]
                   {import,clone,deploy,tag,compile,export} ...
examples.py: error: unrecognized arguments: -v

0xc0170 · 2019-04-17T07:03:22Z

@ARMmbed/mbed-os-test ^^ - this argument was added recently, does this feature branch needs rebase ?

@mprse how old is spi spec?

mprse · 2019-04-17T09:28:39Z

@mprse how old is spi spec?

About 2-3 weeks. Was green for a few days ;-)

0xc0170 · 2019-04-17T13:03:18Z

Can you test locally? If it is all ok, we can restart CI after 5.12.2 release candidate

adbridge · 2019-04-25T15:15:21Z

ci started

mbed-ci · 2019-04-25T22:32:55Z

Test run: FAILED

Summary: 2 of 11 test jobs failed
Build number : 4
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_cloud-client-test
jenkins-ci/mbed-os-ci_greentea-test

adbridge · 2019-04-26T09:58:14Z

@mprse please take a look at the test failures.

mprse · 2019-04-26T12:43:36Z

The results are unexpected. @adbridge can we try again with CI, to check if the results are constant.

adbridge · 2019-04-26T14:39:09Z

ci restarted

mbed-ci · 2019-04-26T18:02:56Z

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 5
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_cloud-client-test

mprse · 2019-04-29T06:06:39Z

We got here mbed-cloud-client-example build error on NUCLEO_F429ZI.

Compile [ 16.2%]: arm_uc_pal_flashiap_implementation.c
[Error] arm_uc_pal_flashiap_implementation.c@577,13: use of undeclared identifier 'MBED_BOOTLOADER_SIZE'
[Error] arm_uc_pal_flashiap_implementation.c@588,43: use of undeclared identifier 'MBED_BOOTLOADER_SIZE'
[Error] arm_uc_pal_flashiap_implementation.c@612,55: use of undeclared identifier 'MBED_BOOTLOADER_SIZE'

I tried to investigate what is causing this failure, so I cloned mbed-cloud-client-example and tried to build it for NUCLEO_F429ZI and got exactly the same failure. This means that the problem is not related to feature-branch. Can anyone more familiar with mbed-cloud-client-example give some hint?

alekla01 · 2019-04-29T10:07:21Z

Restarted cloud-client-test

mprse · 2019-04-29T12:35:50Z

It looks like its ok now!

mprse · 2019-04-30T12:30:04Z

This one is ready to go.

0xc0170 · 2019-04-30T12:37:25Z

This one is ready to go.

I restarted CI before I realized it was waiting for only cloud 🙄

mbed-ci · 2019-04-30T17:15:47Z

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 6
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

adbridge · 2019-05-01T10:32:16Z

Failures seem unrelated to this PR so will restart ci

ciarmcom requested review from ithinuel and a team February 6, 2019 10:00

ciarmcom added the needs: review label Feb 6, 2019

adbridge added needs: work and removed needs: review labels Feb 6, 2019

NirSonnenschein added needs: review and removed needs: work labels Feb 6, 2019

0xc0170 approved these changes Feb 11, 2019

View reviewed changes

0xc0170 added needs: CI release-version: feature branch and removed needs: review labels Feb 11, 2019

ithinuel approved these changes Feb 11, 2019

View reviewed changes

0xc0170 added needs: work and removed needs: CI labels Feb 11, 2019

0xc0170 added needs: CI and removed needs: work labels Feb 19, 2019

mprse force-pushed the SPI_fix branch from c8e1da1 to e88dec2 Compare April 15, 2019 12:21

cmonr added needs: CI and removed needs: work labels Apr 16, 2019

adbridge added needs: work and removed needs: CI labels Apr 26, 2019

0xc0170 added needs: CI and removed needs: work labels Apr 30, 2019

0xc0170 added ready for merge and removed needs: CI labels May 2, 2019

0xc0170 merged commit dc4e6b8 into ARMmbed:feature-hal-spec-spi May 2, 2019

0xc0170 removed the ready for merge label May 2, 2019

SPI::write: fix number of transmitted symbols. #9624

SPI::write: fix number of transmitted symbols. #9624

Uh oh!

Conversation

mprse commented Feb 6, 2019

Description

Pull request type

Reviewers

Uh oh!

jeromecoutant commented Feb 6, 2019

Uh oh!

mprse commented Feb 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ciarmcom commented Feb 6, 2019

Uh oh!

adbridge commented Feb 6, 2019

Uh oh!

mprse commented Feb 6, 2019

Uh oh!

0xc0170 commented Feb 11, 2019

Uh oh!

ithinuel left a comment

Choose a reason for hiding this comment

Uh oh!

mbed-ci commented Feb 11, 2019

Test run: FAILED

Uh oh!

0xc0170 commented Feb 11, 2019

Uh oh!

mprse commented Feb 14, 2019

Uh oh!

0xc0170 commented Feb 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmonr commented Feb 15, 2019

Uh oh!

mprse commented Feb 19, 2019

Uh oh!

0xc0170 commented Feb 19, 2019

Uh oh!

mbed-ci commented Feb 19, 2019

Test run: FAILED

Uh oh!

mprse commented Apr 15, 2019

Uh oh!

mbed-ci commented Apr 17, 2019

Test run: FAILED

Uh oh!

mprse commented Apr 17, 2019

Uh oh!

0xc0170 commented Apr 17, 2019

Uh oh!

mprse commented Apr 17, 2019

Uh oh!

0xc0170 commented Apr 17, 2019

Uh oh!

adbridge commented Apr 25, 2019

Uh oh!

mbed-ci commented Apr 25, 2019

Test run: FAILED

Uh oh!

adbridge commented Apr 26, 2019

Uh oh!

mprse commented Apr 26, 2019

Uh oh!

adbridge commented Apr 26, 2019

Uh oh!

mbed-ci commented Apr 26, 2019

Test run: FAILED

Uh oh!

mprse commented Apr 29, 2019

Uh oh!

alekla01 commented Apr 29, 2019

Uh oh!

mprse commented Apr 29, 2019

Uh oh!

mprse commented Apr 30, 2019

Uh oh!

0xc0170 commented Apr 30, 2019

mprse commented Feb 6, 2019 •

edited

Loading

0xc0170 commented Feb 14, 2019 •

edited

Loading