Hotfix for PR #7778 on Silicon Labs targets #8023

stevew817 · 2018-09-06T19:57:18Z

Description

TB_SENSE_12 was left behind by the changes in #7778. As discussed with @SeppoTakalo over email, this commit implements the changes required to allow TB_SENSE_12 to provide a default network interface for Silicon Labs targets. This was done by mimicking the changes that were put in place by @SeppoTakalo for NCS36510/KW24D.

Considering #7778 made it in for 5.10.0-rc1, I strongly suggest you pull in this PR for the same release to keep continuity. CC @0xc0170 @kjbracey-arm @screamerbg

Pull request type

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

cmonr · 2018-09-06T21:55:27Z

/morph build

mbed-ci · 2018-09-06T23:00:26Z

Build : FAILURE

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

cmonr · 2018-09-07T03:28:49Z

@stevew817 We'll do what we can to get this in for RC2 (currently slipping to Monday), but it looks like this still needs some work with TB_Sense_1

stevew817 · 2018-09-07T08:11:21Z

@SeppoTakalo TB_SENSE_1 is failing because some (netsocket) tests do not fit in its memory. What's the recommended course of action to turn off these tests?

0xc0170 · 2018-09-07T08:49:42Z

@SeppoTakalo TB_SENSE_1 is failing because some (netsocket) tests do not fit in its memory. What's the recommended course of action to turn off these tests?

@ARMmbed/mbed-os-test Please review

SeppoTakalo · 2018-09-07T09:30:56Z

Recommended way is to have:

    "TB_SENSE_12": {
        "default_test_configuration": "NO_NETWORK",

in the test configuration, in case test cases do not fit.

But this is already provided, so I don't understand why it does not build...

SeppoTakalo · 2018-09-07T09:33:00Z

$ mbed test --compile -n mbed-os-tests-netsocket\*
...
Compile [ 73.3%]: main.cpp
[Error] main.cpp@21,2: #error [NOT_SUPPORTED] No network configuration found for this target.
...
Build successes:
  * TB_SENSE_12::GCC_ARM::MBED-BUILD


Build skips:
  * TB_SENSE_12::GCC_ARM::MBED-OS-TESTS-NETSOCKET-DNS
  * TB_SENSE_12::GCC_ARM::MBED-OS-TESTS-NETSOCKET-TCP
  * TB_SENSE_12::GCC_ARM::MBED-OS-TESTS-NETSOCKET-UDP

This is what happens on my machine, so I don't understand why the skipping does not work on CI

0xc0170 · 2018-09-07T10:38:09Z

Enable these -DMBED_HEAP_STATS_ENABLED=1 -DMBED_STACK_STATS_ENABLED=1 -DMBED_TRAP_ERRORS_ENABLED=1 -DMBED_ALL_STATS_ENABLED - might do the difference.

stevew817 · 2018-09-07T11:13:21Z

Please keep in mind that it is TB_SENSE_1 which is failing, not TB_SENSE_12. TB_SENSE_1 is a target based on EFR32MG1P which has 256k flash/64k ram, so I wouldn't be surprised if the stack doesn't fit in there... However, it used to work with mesh-minimal-example, so...
@SeppoTakalo Any suggestions?

SeppoTakalo · 2018-09-07T12:21:48Z

@stevew817 Please provide the test config for TB_SENSE_1 as well, if both have 802.15.4 RF enabled.

Mesh application might fit, if configured to be minimal enough, but same does not apply to our test applications which might use full set of Socket operations, DNS libraries, etc.

So in this first phase, provide the support for interface with test configurations disabled.
Then later on we will work out how to get mesh boards into test.

Also, another reason to use the NO_NETWORK config in test, is that the CI that runs these PR tests, is not located in place where mesh network is available. We have separate CI for that.

So, add:

     },
    "TB_SENSE_1": {
        "default_test_configuration": "NO_NETWORK",
        "test_configurations": ["6LOWPAN_HOST", "6LOWPAN_ROUTER", "THREAD_END_DEVICE", "THREAD_ROUTER"]
    }

to the target_configs.json as well.

stevew817 · 2018-09-07T14:13:40Z

@SeppoTakalo @0xc0170 @cmonr fixed.

cmonr · 2018-09-07T15:01:51Z

/morph build

mbed-ci · 2018-09-07T16:21:57Z

Build : SUCCESS

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

Triggering tests

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

mbed-ci · 2018-09-07T18:33:57Z

Test : FAILURE

Build number : 2803
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8023/2803

stevew817 · 2018-09-07T18:43:03Z

Unrelated failure...

cmonr · 2018-09-07T18:43:37Z

Correct, both are not.
/morph test

mbed-ci · 2018-09-07T19:08:57Z

Exporter Build : FAILURE

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

kjbracey · 2018-09-20T11:27:57Z

This can't be the only target that is suffering from such a generic failure mode???

At the minute, it does seem to be. I think you've been spectacularly unlucky here.

The fundamental problem in question is fairly recent - apparently an IAR assembler bug triggered by the complicated (and wrongly-quoted) new error message config option that's recently been added to JSON.

There must be some lingering instability not addressed by the previous workaround, and somehow your xcl file manages to achieve some sort of failure (on 7.80.1, but not 7.80.4?). Presumably the same thing would pop up on other targets later if they changed something about their config - it seems deterministic for a particular config.

I've done a bit more local testing - it seems that "" is the correct substitution to get a quote into the assembler to define a string, so "-Dstr=""hello""". However, that doesn't stop "-Dstr=""http://""" from exploding 7.80.2. Substituting that to make "-Dstr=""http:/\/""" does feed in the correct string with no crash, at the cost of an "unrecognised escape sequence" warning if the string is used.

However, we still don't know what exactly it is that is upsetting just this particular target. I will try that substitution, to see if maybe adding the quotes back avoids some other problem with the \n or something. (Not that \n is a valid escape for the assembler either...)

But given that we already know the -11 error is target-specific, maybe /any/ change we made to the xcl file would make it go away like for the other targets. It seems like the safest bet is to err on the side of caution and eliminate all string macros, hoping and assuming that this -11 is also something about string macros.

kjbracey · 2018-09-20T11:32:04Z

/morph mbed2-build

kjbracey · 2018-09-20T12:45:59Z

And, that fails. Okay, should I make a separate PR for the string macro strip out, or fold it into this one?

It's logically separate, but CI load is high, and there's already loads of discussion here.

0xc0170 · 2018-09-20T12:50:33Z

It's logically separate, but CI load is high, and there's already loads of discussion here.

In this case here to make it green

IAR assembler 7.80 has some problems handling difficult macros, leading to immediate exit with return value -11. In particular, a URL string has been causing problems, presumably due to the "//" resembling a comment. A previous escaping workaround in 0d97803 seemed to work, but the crash has still been seen with a particular target. Previous creation of the extended command line file for the IAR assembler was stripping quotes from macros. This rendered the resulting definitions for string-containing macros incorrect, which means that we can assume no assembler code is currently relying on them. Therefore, as a precautionary measure to avoid the crash, simply remove all macros containing strings when creating them for IAR. This apparently clears the crashes seen during testing of ARMmbed#8023

kjbracey · 2018-09-20T13:33:38Z

@theotherjimmy and @TTornblom could you review the last commit messing with IAR assembler, please?

0xc0170 · 2018-09-20T13:54:04Z

/morph mbed2-build

TTornblom · 2018-09-20T15:10:11Z

I have been playing some with the macro rewrite, and here is an alternative to remove the double quotes that seems to work. It will leave the double quotes in, but "//" still needs to be rewritten as "/\/".

diff --git a/tools/toolchains/iar.py b/tools/toolchains/iar.py
index b54487869..ed93dee1c 100644
--- a/tools/toolchains/iar.py
+++ b/tools/toolchains/iar.py
@@ -167,7 +167,7 @@ class IAR(mbedToolchain):
         opts = ['-D%s' % d for d in defines]
         if for_asm:
             config_macros = self.config.get_config_data_macros()
-            macros_cmd = ['"-D%s"' % d.replace('"', '').replace('//','/\/') for d in config_macros]
+            macros_cmd = ['"-D%s"' % d.replace('"', '""').replace('//','/\/') for d in config_macros]
             if self.RESPONSE_FILES:
                 via_file = self.make_option_file(
                     macros_cmd, "asm_macros_{}.xcl")

cmonr · 2018-09-20T17:57:49Z

The plan:

I'm going to take @TTornblom's patch and commit it to this PR. If the mbed2-build job passes with it, then we'll progress the PR. If not, then I'll revert the PR, progress the PR asis, and open a new issue to look into why the patch didin't work.

cmonr · 2018-09-20T18:03:44Z

/morph mbed2-build

cmonr · 2018-09-20T18:37:41Z

Welp, since that didn't work, backing out the commit and progressing the PR with only @kjbracey-arm's change.

cmonr · 2018-09-20T18:40:43Z

/morph mbed2-build

cmonr · 2018-09-20T19:13:41Z

/morph build

mbed-ci · 2018-09-20T20:37:17Z

Build : SUCCESS

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

Triggering tests

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

mbed-ci · 2018-09-20T23:02:36Z

Exporter Build : SUCCESS

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

mbed-ci · 2018-09-21T09:06:33Z

Test : FAILURE

Build number : 2915
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8023/2915

0xc0170 · 2018-09-21T09:10:37Z

/morph test

mbed-ci · 2018-09-21T16:51:29Z

Test : SUCCESS

Build number : 2919
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8023/2919

cmonr · 2018-09-21T17:50:40Z

Phew. That took waaay too long.

IAR assembler 7.80 has some problems handling difficult macros, leading to immediate exit with return value -11. In particular, a URL string has been causing problems, presumably due to the "//" resembling a comment. A previous escaping workaround in 0d97803 seemed to work, but the crash has still been seen with a particular target. Previous creation of the extended command line file for the IAR assembler was stripping quotes from macros. This rendered the resulting definitions for string-containing macros incorrect, which means that we can assume no assembler code is currently relying on them. Therefore, as a precautionary measure to avoid the crash, simply remove all macros containing strings when creating them for IAR. This apparently clears the crashes seen during testing of #8023

cmonr added the release-version: 5.10.0-rc2 label Sep 6, 2018

cmonr requested review from SeppoTakalo, 0xc0170, kjbracey and screamerbg September 6, 2018 21:54

cmonr added the needs: review label Sep 6, 2018

cmonr approved these changes Sep 6, 2018

View reviewed changes

cmonr added needs: work and removed needs: review labels Sep 7, 2018

SeppoTakalo approved these changes Sep 7, 2018

View reviewed changes

kjbracey approved these changes Sep 7, 2018

View reviewed changes

0xc0170 approved these changes Sep 7, 2018

View reviewed changes

SeppoTakalo approved these changes Sep 7, 2018

View reviewed changes

kjbracey force-pushed the hotfix/nanostack-default-interface branch from 3d0e2e4 to 0e7eda0 Compare September 20, 2018 13:17

cmonr removed the needs: preceding PR label Sep 20, 2018

cmonr force-pushed the hotfix/nanostack-default-interface branch from eb2af03 to 0e7eda0 Compare September 20, 2018 18:40

cmonr merged commit caa4279 into ARMmbed:master Sep 21, 2018

cmonr removed the needs: CI label Sep 21, 2018

kjbracey mentioned this pull request Dec 3, 2018

IAR support broken with Pelion DM Client during ASM #8931

Closed

Hotfix for PR #7778 on Silicon Labs targets #8023

Hotfix for PR #7778 on Silicon Labs targets #8023

Conversation

stevew817 commented Sep 6, 2018

Description

Pull request type

cmonr commented Sep 6, 2018

mbed-ci commented Sep 6, 2018

cmonr commented Sep 7, 2018

stevew817 commented Sep 7, 2018

0xc0170 commented Sep 7, 2018

SeppoTakalo commented Sep 7, 2018

SeppoTakalo commented Sep 7, 2018

0xc0170 commented Sep 7, 2018

stevew817 commented Sep 7, 2018

SeppoTakalo commented Sep 7, 2018 • edited Loading

stevew817 commented Sep 7, 2018

cmonr commented Sep 7, 2018

mbed-ci commented Sep 7, 2018

Triggering tests

mbed-ci commented Sep 7, 2018

stevew817 commented Sep 7, 2018

cmonr commented Sep 7, 2018

mbed-ci commented Sep 7, 2018

kjbracey commented Sep 20, 2018

kjbracey commented Sep 20, 2018

kjbracey commented Sep 20, 2018

0xc0170 commented Sep 20, 2018

kjbracey commented Sep 20, 2018

0xc0170 commented Sep 20, 2018

TTornblom commented Sep 20, 2018 • edited by cmonr Loading

cmonr commented Sep 20, 2018

cmonr commented Sep 20, 2018

cmonr commented Sep 20, 2018

cmonr commented Sep 20, 2018

cmonr commented Sep 20, 2018

mbed-ci commented Sep 20, 2018

Triggering tests

mbed-ci commented Sep 20, 2018

mbed-ci commented Sep 21, 2018

0xc0170 commented Sep 21, 2018

mbed-ci commented Sep 21, 2018

cmonr commented Sep 21, 2018

SeppoTakalo commented Sep 7, 2018 •

edited

Loading

TTornblom commented Sep 20, 2018 •

edited by cmonr

Loading