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

add support for SILICA_SENSOR_NODE platform #5104

Merged
merged 12 commits into from Oct 19, 2017

Conversation

Projects
None yet
9 participants
@architech-boards

architech-boards commented Sep 14, 2017

Tests done with 3 toolchains on Windows:

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 14, 2017

@architech-boards It looks like you based this on a release. We can only accept pull requests (PRs) based on master. Please rebase your PR onto master.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 14, 2017

@architech-boards While your link is correct, "uVision 5.24.2.0" is not the IDE or version of IAR.

@architech-boards

This comment has been minimized.

architech-boards commented Sep 14, 2017

You are right, IAR is 7.80.2.11975.
I'm going to rebase it

@architech-boards

This comment has been minimized.

architech-boards commented Sep 15, 2017

@theotherjimmy I rebased my commits, I'm not sure if it is ok now, commands used:
git remote add upstream git://github.com/ARMmbed/mbed-os.git
git fetch upstream
git rebase upstream/master
git push -f origin master

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 15, 2017

@architech-boards It looks like yo did a rebase correctly. I still see Anna's "Added versioning define block for new minor release." commit, which is what originally indicated to me that you based this branch on a release (she only adds that commit, and others like it, to the release branches).

@architech-boards

This comment has been minimized.

architech-boards commented Sep 15, 2017

Yes exactly, I don't know how to remove Annas's commit. I tried but unsuccessfully. Thanks.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 15, 2017

You could do an interactive rebase (git rebase -i origin/master) and just delete the line related to that commit.

@@ -31,7 +31,7 @@
/* Heap for NanoStack */
#if !MBED_CONF_MBED_MESH_API_USE_MALLOC_FOR_HEAP
static uint8_t app_stack_heap[MBED_CONF_MBED_MESH_API_HEAP_SIZE + 1];
static uint8_t app_stack_heap[MBED_CONF_MBED_MESH_API_HEAP_SIZE + 1] __attribute__(( section( "mesh_heap") ));

This comment has been minimized.

@0xc0170

0xc0170 Sep 20, 2017

Member

What is this change? I believe this section is not defined for all targets. Or is this valid?

This comment has been minimized.

@architech-boards

architech-boards Sep 20, 2017

L476JG memory is splitted in 2 banks with not consecutive addresses. In the original project only 96kb SRAM1 is used but this memory is not enought for the project. We moved this array to the 32kb SRAM2 bank which the original project don't use. See the STM32L476XX.ld file modified.

This comment has been minimized.

@0xc0170

0xc0170 Sep 21, 2017

Member

This is target specific then, should not be in this code, you should be able to place object files in the sections (might be better place in the linker to do this than in this generic file).

@SeppoTakalo please review this

This comment has been minimized.

@architech-boards

architech-boards Sep 21, 2017

I changed the STM32L476XX.ld file with:

.mesh_heap (COPY):
{
    *mesh_system.o
} > SRAM2

and removed attribute(( section( "mesh_heap") ));
The bin file is built without errors and the .map files seems good: mbed-os-sensor-node.map.txt

but when I compile this mbed-os version for the mbed-os-example-client the demo doesn't run:

Starting mbed Client example
[EasyConnect] IPv6 mode
[EasyConnect] Using Mesh
[EasyConnect] Connecting to Mesh..
[EasyConnect] MAC address
[EasyConnect] Connection to Network Failed -3012!
Connection to Network Failed - exiting application...

Please can you help me?

This comment has been minimized.

@0xc0170

This comment has been minimized.

@JanneKiiskila

JanneKiiskila Sep 22, 2017

Contributor

@SeppoTakalo might be able to help out a bit more, but well - more logs would definitely be needed. Easy-connect itself does nothing there, it's just abstracting the network stack differences into one function we call from the app side.

@SeppoTakalo - we have one of those boards available here in Oulu.

This comment has been minimized.

@JanneKiiskila

JanneKiiskila Sep 22, 2017

Contributor

Please note that there is some funny thing on-going with the MAC address. In all normal scenarios the MAC address printing should work - however, in case - there is no MAC? Is the radio driver working?

This comment has been minimized.

@architech-boards

architech-boards Sep 25, 2017

Please note that there is some funny thing on-going with the MAC address.
Yes it's true. In the mbed_app.json I have: "spirit1.mac-address": "{0x23, 0x23, 0x23, 0x23, 0x23, 0x23, 0x23, 0x23}"

This with trace enabled:

Starting mbed Client example
[EasyConnect] IPv6 mode
[EasyConnect] Using Mesh
[EasyConnect] Connecting to Mesh..
[DBG ][SPIRIT]: rf_register (728)
[DBG ][SPIRIT]: rf_ack_loop (608)
[DBG ][SPIRIT]: rf_register (775)
[EasyConnect] MAC address
[EasyConnect] Connection to Network Failed -3012!

Connection to Network Failed - exiting application...

If I comment this (just for test):
/*.mesh_heap (COPY):
{
mesh_system.o
} > SRAM2
/
from the .ld file, the radio works:

Starting mbed Client example
[EasyConnect] IPv6 mode
[EasyConnect] Using Mesh
[EasyConnect] Connecting to Mesh..
[DBG ][core]: Allocate Root Tasklet
[DBG ][6lo ]: P.Init

[DBG ][core]: NS Root task Init
[DBG ][sck ]: Socket Tasklet Generated
[DBG ][SPIRIT]: rf_register (728)
[DBG ][SPIRIT]: rf_ack_loop (608)
[DBG ][SPIRIT]: rf_interface_state_control (466)
[DBG ][SPIRIT]: rf_register (775)
[DBG ][SPIRIT]: get_mac_address (797)
[DBG ][SPIRIT]: rf_get_mac_address (559), adr0=23, adr1=23, adr2=23, adr3=23, adr4=23, adr5=23, adr6=23, adr7=23
[DBG ][SPIRIT]: rf_set_mac_address (547), adr0=23, adr1=23, adr2=23, adr3=23, adr4=23, adr5=23, adr6=23, adr7=23
[DBG ][SPIRIT]: rf_set_mac_address (547), adr0=23, adr1=23, adr2=23, adr3=23, adr4=23, adr5=23, adr6=23, adr7=23
[DBG ][nslp]: connect()
[DBG ][mle ]: MLE service init size 32
[DBG ][sck ]: Socket id 0 allocated
[DBG ][m6LND]: Using PSK security mode.
[DBG ][m6LND]: Channel: 1
[DBG ][m6LND]: Channel page: 2
[DBG ][m6LND]: Channel mask: 2
[DBG ][mMIB]: Set key 1
[INFO][addr]: Address added to IF 1: fe80::2123:2323:2323:2323

But without my original modification the client won't connect to the mbed connector due to memory overflow problems.

This comment has been minimized.

@0xc0170

0xc0170 Oct 2, 2017

Member

I believe we all agree this change should not be here, and we need to identify the root cause of this problem.

@JanneKiiskila you noted that you have these boards in Oulu, or @MarceloSalazar in Cambridge to reproduce the problems they are having?

mbed.h Outdated
#endif
#define MBED_ENCODE_VERSION(major, minor, patch) ((major)*10000 + (minor)*100 + (patch))
#define MBED_VERSION MBED_ENCODE_VERSION(MBED_MAJOR_VERSION, MBED_MINOR_VERSION, MBED_PATCH_VERSION) #define MBED_H

This comment has been minimized.

@0xc0170

0xc0170 Sep 20, 2017

Member

As pinpointed earlier, please rebase interactively to remove this change

This comment has been minimized.

@architech-boards

architech-boards Sep 20, 2017

thank you, rebase done, removed the commit

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Sep 20, 2017

@architech-boards Let us know if you need any assistance with rebase

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Sep 22, 2017

This seems to add new platform, but what is the radio driver?
How can we study the problem if we don't know which radio it failed.
Also, if the work is to port a new radio driver, try starting with the https://github.com/ARMmbed/mbed-os-example-mesh-minimal application instead of Client. It is much easier to debug.

@MarceloSalazar

This comment has been minimized.

Contributor

MarceloSalazar commented Sep 23, 2017

@architech-boards could you please share links to your forked mesh-minimal and client application (pointing at this PR mbed OS branch) to help us investigate further?

@architech-boards

This comment has been minimized.

architech-boards commented Sep 25, 2017

This seems to add new platform, but what is the radio driver?
Why new radio driver? We are using stm-spirit1-rf-driver.git from mbed-os-example-client.
If you want see a working version compilable for our board you can download our repository:

hg clone https://AVNETSilica@os.mbed.com/users/AVNETSilica/code/mbed-os-sensor-node/

This is a porting from mbed-os-example-client. It uses the last mbed-os version.

could you please share links to your forked mesh-minimal and client application (pointing at this PR mbed OS branch) to help us investigate further?
We have only this repo: mbed-os-sensor-node and it is the porting from mbed-os-example-client.

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Sep 25, 2017

@architech-boards
Thanks for the info.
I'm having trouble understanding why do you fork the Client repository if your changes are going to land on the Mbed OS?

Also, what you linked, points to private clone of Easy-connect. However it links to main Spirit RF driver with quite new Git hash, so it seems OK.

  • Have you ever been able to successfully create mesh network with these nodes?
  • What border router are you using? is is running and connected to IPv6 network. Are you able to provide log file from its serial output.
  • Do you have any RF sniffer that is able to capture packets between these nodes and border router?
  • Can you use mesh-minimal example instead so we can first debug the mesh issue and then IPv6 connectivity issues?
  • From the mbed_app.json:
    • Can you enable Nanostack's debug traces by turning on the "mbed-trace.enable": true option
    • Use feature NANOSTACK_FULL in the "target.features_add" line instead of LOWPAN_ROUTER? to get all debug prints.
@architech-boards

This comment has been minimized.

architech-boards commented Sep 25, 2017

  • I'm having trouble understanding why do you fork the Client repository if your changes are going to land on the Mbed OS?
    This repository is for our seminars. So customers can download it and try to run it on the board. In this repository there is also "extra source code" just for the hands-on (at mbed-os-sensor-node\mbed-os\targets\TARGET_STM\TARGET_STM32L4\TARGET_STM32L476xG\TARGET_SILICA_SENSOR_NODE\SDK).

  • Also, what you linked, points to private clone of Easy-connect.
    Yes because we have to reset the spirit device before to call the constructor stm-spirit1-rf-driver. So I created a new class just to reset the spirit and then call the stm-spirit1-rf-driver constructor. This because I can't modify directly the stm-spirit1-rf-driver source code.

  • Have you ever been able to successfully create mesh network with these nodes?
    Yes with 20 nodes connected contemporaneously, wtithout problems.

  • What border router are you using? is is running and connected to IPv6 network.
    Yes, We are using the NUCLEO_F429ZI board with the SPIRIT module X-NUCLEO-IDS01A4. I modified the nanostack-border-router repository just to be able to connect the 20 nodes without any problem. You can find a guide here (with the firmware binary downloadable): http://sensor-node.readthedocs.io/en/latest/demo.html (ps. I have to correct the English)
    Sure, Ipv6.

  • Are you able to provide log file from its serial output.
    Yes, soon I'm going to send the file.

  • Do you have any RF sniffer that is able to capture packets between these nodes and border router?
    yes

  • Can you use mesh-minimal example instead so we can first debug the mesh issue and then IPv6 connectivity issues?
    I never used it. But we have not created a new radio driver. So why debug the mesh issue? The mesh works if I don't change the .ld file with the attribute: *mesh_system.o. I think this problem is about linker and memory map not the radio driver directly.

  • From the mbed_app.json:
    ok if you want it.

Please, try to compile my repository, you will see that the mesh works. And the clients connects to the mbed connector. I think the problem is about .ld file.

@architech-boards

This comment has been minimized.

architech-boards commented Sep 28, 2017

Hello, I forked and changed 2 repositories as requested. These repos work with our board.

https://github.com/architech-boards/mbed-os-example-mesh-minimal
https://github.com/architech-boards/mbed-os-example-clien

nb. use my json file from the "configs" directory

@JanneKiiskila

This comment has been minimized.

Contributor

JanneKiiskila commented Sep 28, 2017

We'll get that new config added to the mbed-os-example-client repo easily. When can we have the required mbed-os changes in?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 2, 2017

I would like to move this target along. As we are having problem with client, do you think we should split this (simplify) patch? The nanostack change should not be here as noted previously. @architech-boards what do you think?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 6, 2017

I would like to move this target along. As we are having problem with client, do you think we should split this (simplify) patch? The nanostack change should not be here as noted previously. @architech-boards what do you think?

Any update? Please can yo uresolve conflict (rebase will also eliminate circle-ci failure plus possibly jenkins cI).

What is the status of client failures with linker script update to place mesh in the second RAM bank?

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Oct 6, 2017

Where are the linker files for IAR and ARM toolchains?

This adds support only for GCC.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 6, 2017

Where are the linker files for IAR and ARM toolchains?

It inherits from STM32L476JG thus getting the same support (all 3 toolchains), verified via targats.json file edit here and files for STM32L476xG.

This means that whatever change is needed for that mesh, needs to be done in all 3 linker files (better organization of 2 RAM banks) for connectivity

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Oct 6, 2017

Then why do they provide linker file for GCC instead of inheriting it also?

Getting target to work with Mbed OS is one goal, getting enough RAM to run it with mesh application is another goal.

These two should be handled in separate pull requests.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 6, 2017

Getting target to work with Mbed OS is one goal, getting enough RAM to run it with mesh application is another goal.

Agree, the same proposal I made above. @architech-boards Please see the latest comments, and tell us what you think.

Then why do they provide linker file for GCC instead of inheriting it also?

Not certain here. there could be then 2 linker script files here. @architech-boards Can you review the inheritance, and which linker scripts are provided, clarify please.

@architech-boards

This comment has been minimized.

architech-boards commented Oct 6, 2017

Getting target to work with Mbed OS is one goal, getting enough RAM to run it with mesh application is another goal.
Agree, the same proposal I made above. @architech-boards Please see the latest comments, and tell us what you think.

We have talked yesterday with @MarceloSalazar and we agree about remove our LD file and the attribute in the mesh_system.c. Our priority now is get the target to work with Mbed OS. So getting enought RAM to run it with mesh application is not important at this point.
Have I to create a new commit that removes these changes?

Then why do they provide linker file for GCC instead of inheriting it also?
Not certain here. there could be then 2 linker script files here. @architech-boards Can you review the inheritance, and which linker scripts are provided, clarify please.

@MarceloSalazar knows the details about the memory problem because yesterday we have talked with him about why there is this problem. I think he will talk to you internally.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 9, 2017

Have I to create a new commit that removes these changes?

First, rebase your branch on top of latest master (resolve the conflict that is here). look at ``git rebase master` once master is up to date (your local master = upstream/master).

You can then just create a new commit to revert that nanostack change, or use git revert for that specific commit that is adding that attribute (if it is in a separete commit).

@architech-boards

This comment has been minimized.

architech-boards commented Oct 11, 2017

I rebased it and I created a new commit to revert that nanostack change. For me it is ready to be reviewed and merged. Thank you!

@mbed-ci

This comment has been minimized.

mbed-ci commented Oct 11, 2017

Build : SUCCESS

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

Triggering tests

/test mbed-os

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 13, 2017

Changes look good to me now. Waiting for CI to complete

@studavekar

This comment has been minimized.

Collaborator

studavekar commented Oct 16, 2017

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Oct 16, 2017

Build : SUCCESS

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

Skipping test trigger, missing label 'NEED CI'

@adbridge adbridge added needs: CI and removed needs: work labels Oct 16, 2017

@mbed-ci

This comment has been minimized.

mbed-ci commented Oct 16, 2017

Build : SUCCESS

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

Triggering tests

/test mbed-os

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 17, 2017

/morph test

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 18, 2017

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Oct 18, 2017

Build : SUCCESS

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

Triggering tests

/morph test
/morph uvisor-test

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 18, 2017

/morph uvisor-test

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 merged commit 2af7213 into ARMmbed:master Oct 19, 2017

7 checks passed

AWS-CI uVisor Build & Test Success
Details
AWS-CI uvisor Test DIDNT RUN UVISOR TESTS
Details
Cam-CI uvisor Build & Test DIDNT RUN UVISOR TESTS
Details
ci-morph-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment