Skip to content

Add back examples updated for mbed-os-5.10 #8246

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

Merged
merged 6 commits into from
Sep 28, 2018
Merged

Conversation

adbridge
Copy link
Contributor

Description

While feature changes were being added to mbed-os a number of
examples were removed from this list due to incompatibility issues.
This PR adds those examples back in:
Filesystem,
Blockdevice,
Mesh-minimal,
Bootloader.

This PR also adds in the new NFC example.

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change

While feature changes were being added to mbed-os a number of
examples were removed from this list due to incompatibility issues.
This PR adds those examples back in:
Filesystem,
Blockdevice,
Mesh-minimal,
Bootloader.

This PR also adds in the new NFC example.
@cmonr
Copy link
Contributor

cmonr commented Sep 25, 2018

@adbridge Two things:

  1. A lot of the examples are only built against the K64F, leading to longer build times for just that target. Do we want to consider targeting something different so that the risk of having K64F builds timing out remains low?
  2. If a target is not specified, the example is built against all available targets (in reference to this: https://github.com/ARMmbed/mbed-os/pull/8246/files#diff-93b92c61c9977ce2503bf0c37f41a92dR303). Is that what we want for the NFC example?

@cmonr
Copy link
Contributor

cmonr commented Sep 25, 2018

@ARMmbed/mbed-os-maintainers ^^^

@adbridge
Copy link
Contributor Author

@cmonr In terms of 1. That is what is specified by the example owners so we shouldn't change that unless they say so. In terms of 2. I will ask Vincent to review.
@pan- @donatieng Can you please comment on the compilation for the NFC example.

Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

This is the revert of the removal earlier - LGTM

As @cmonr stated, we should review this for 5.11. @OPpuolitaival If not yet in, we should review example testing

"name": "mbed-os-example-nfc",
"github": "https://github.com/ARMmbed/mbed-os-example-nfc",
"mbed": [
"https://os.mbed.com/teams/mbed-os-examples/code/mbed-os-example-nfc-EEPROM/",
Copy link
Member

@pan- pan- Sep 26, 2018

Choose a reason for hiding this comment

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

That example is restricted to NUCLEO_F401RE and the DISCO_L475VG_IOT01A at the current minute.

Copy link
Contributor

@0xc0170 0xc0170 Sep 26, 2018

Choose a reason for hiding this comment

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

That example is restricted to NUCLEO_F401RE and the DISCO_L475VG_IOT01A at the current minute.

Not certain if this one commit just adds back what used to be there or adds something new. Should be clarified and fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

@0xc0170 This PR is doing both. NFC is a new example that was introduced with 5.10.

],
"test-repo-source": "mbed",
"features" : [],
"targets" : [],
Copy link
Contributor

Choose a reason for hiding this comment

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

That really looks that it does not work for all devices as configured now

@cmonr
Copy link
Contributor

cmonr commented Sep 26, 2018

@pan- @donatieng Mind taking another look? I've restricted the NFC example to the two specified targets.

Copy link
Member

@pan- pan- left a comment

Choose a reason for hiding this comment

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

Approved for NFC.

@cmonr
Copy link
Contributor

cmonr commented Sep 26, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Sep 26, 2018

Build : FAILURE

Build number : 3169
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8246/

@cmonr
Copy link
Contributor

cmonr commented Sep 26, 2018

Going to retry. Not sure why the examples failed to build.
/morph build

@mbed-ci
Copy link

mbed-ci commented Sep 26, 2018

Build : FAILURE

Build number : 3171
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8246/

@cmonr
Copy link
Contributor

cmonr commented Sep 26, 2018

The issue is subtle, but it looks like the build-prep step is failing to clone the nfc examples.

This apparently breaks the examples.py script.
@cmonr
Copy link
Contributor

cmonr commented Sep 26, 2018

/morph build

@cmonr
Copy link
Contributor

cmonr commented Sep 26, 2018

For reference, it appears that having the trailing slash was causing the following error:

18:08:57 '' example directory doesn't exist. Skipping...
18:08:57 '' example directory doesn't exist. Skipping...

However, the script continues without failing.

@mbed-ci
Copy link

mbed-ci commented Sep 26, 2018

Build : FAILURE

Build number : 3172
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8246/

@studavekar
Copy link
Contributor

Build : FAILURE

Build number : 3172
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8246/

Looks like legit build failures

  1. atmel-rf' is not unique
ConfigException: Library name 'atmel-rf' is not unique (defined in '/builds/ws/mbed-os-build-matrix/3172/K64F_GCC_ARM/sources/examples/mbed-os-example-tls-tls-client/easy-connect/atmel-rf-driver/mbed_lib.json' and '/builds/ws/mbed-os-build-matrix/3172/K64F_GCC_ARM/sources/examples/mbed-os-example-tls-tls-client/mbed-os/components/802.15.4_RF/atmel-rf-driver/mbed_lib.json')
  1. Output: ./main.cpp:41:1: error: 'SPIFBlockDevice' does not name a type
[DEBUG] Output: ./main.cpp:41:1: error: 'SPIFBlockDevice' does not name a type
[DEBUG] Output:  SPIFBlockDevice bd(
[DEBUG] Output:  ^~~~~~~~~~~~~~~
[DEBUG] Output: ./main.cpp: In function 'void erase()':
[DEBUG] Output: ./main.cpp:56:15: error: 'bd' was not declared in this scope
[DEBUG] Output:      int err = bd.init();
[DEBUG] Output:                ^~
[DEBUG] Output: ./main.cpp: In function 'int main()':
[DEBUG] Output: ./main.cpp:91:25: error: 'bd' was not declared in this scope
[DEBUG] Output:      int err = fs.mount(&bd);
[DEBUG] Output:                          ^~

@cmonr
Copy link
Contributor

cmonr commented Sep 27, 2018

Pinging @ARMmbed/mbed-os-storage (and @geky) for the filesystem example.

@ARMmbed/mbed-os-maintainers Who do we ping for mbed-os-example-tls-tls-client?

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 27, 2018

@ARMmbed/mbed-os-maintainers Who do we ping for mbed-os-example-tls-tls-client?

Can you review the failures above @k-stachowiak @Patater

Pinging @ARMmbed/mbed-os-storage (and @geky) for the filesystem example.

I suspect most of the storage team is OoO today, @geky can you help in case we do not receive a response here (I contacted them privately as well)

@k-stachowiak
Copy link
Contributor

@0xc0170 In mbed-os-example-tls we need this merged: ARMmbed/mbed-os-example-tls#198 It is pending minor changes from @SeppoTakalo. Please let me know if we may do something to support him.

@cmonr
Copy link
Contributor

cmonr commented Sep 27, 2018

Fingers crossed this does the trick. Otherwise, I'm going to keep those two storage examples disabled and bring this in without them to get 5.10.1 generated tonight.

/morph build

(I say fingers crossed because even after changing targets, the build still failed locally, similar to this failure: #8246 (comment))

@deepikabhavnani
Copy link

K64F": {
"supported_form_factors": ["ARDUINO"],
"components": ["SD"],

Similar to this all storage components should be present in K82F else will get build failures

@mbed-ci
Copy link

mbed-ci commented Sep 28, 2018

Build : FAILURE

Build number : 3185
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8246/

@cmonr
Copy link
Contributor

cmonr commented Sep 28, 2018

Well darn. Looks like the block device example still needs some work.

Going to progress this without it for now.

/morph build

@mbed-ci
Copy link

mbed-ci commented Sep 28, 2018

Build : SUCCESS

Build number : 3186
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8246/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Sep 28, 2018

@mbed-ci
Copy link

mbed-ci commented Sep 28, 2018

@NirSonnenschein
Copy link
Contributor

/morph export-build

@mbed-ci
Copy link

mbed-ci commented Sep 28, 2018

@NirSonnenschein
Copy link
Contributor

This PR is not passing export due to failures finding printf on the following boards (failure occurs on all toolchains but only occurs on two boards ).

  1. mbed-os-example-nfc-EEPROM DISCO_L475VG_IOT01A on DISCO_L475VG_IOT01A
  2. mbed-os-example-nfc-EEPROM NUCLEO_F401RE on NUCLEO_F401RE
    @donatieng and @pan- @adbridge has asked that you please take a look. Note: this is the LAST remaining PR for 5.10.1 which is supposed to be built later today.

also CCing @cmonr in case this runs into your shift.

Last thing blocking 5.10.1 release. Example will need to be fixed before it can be brought in.
@cmonr
Copy link
Contributor

cmonr commented Sep 28, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Sep 28, 2018

Build : SUCCESS

Build number : 3187
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8246/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Sep 28, 2018

@cmonr
Copy link
Contributor

cmonr commented Sep 28, 2018

/morph test

@mbed-ci
Copy link

mbed-ci commented Sep 28, 2018

@mbed-ci
Copy link

mbed-ci commented Sep 28, 2018

@cmonr cmonr merged commit 255f28e into ARMmbed:master Sep 28, 2018
@cmonr cmonr removed the needs: CI label Sep 28, 2018
0xc0170 pushed a commit that referenced this pull request Oct 1, 2018
adbridge pushed a commit that referenced this pull request Oct 8, 2018
adbridge pushed a commit that referenced this pull request Oct 8, 2018
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.