Skip to content

Add TARGET_STMBLUE #9491

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

Closed
wants to merge 49 commits into from
Closed

Add TARGET_STMBLUE #9491

wants to merge 49 commits into from

Conversation

ntoni92
Copy link

@ntoni92 ntoni92 commented Jan 24, 2019

Description

This pull request contains files necessary to the execution of Mbed-OS 5.11 over STM BlueNRG2 SoC. This porting includes USTICKER, GPIO, UART and BLE Cordio stack.
Mbed OS 5.11 works on BlueNRG2 only without RTOS (SRAM is not enough). The porting has been tested with ARM BLE HRM example, modified to use a shared queue and public address. It has been compiled with GCC_ARM with -Og option, otherwise code size exceeds flash size. In this configuration the use of standard printf is not allowed.

Pull request type

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

Reviewers

@donatieng
@pan-
@avilei
@apalmieriGH

@ciarmcom ciarmcom requested review from donatieng, pan- and a team January 24, 2019 18:00
@ciarmcom
Copy link
Member

@ntoni92, thank you for your changes.
@donatieng @pan- @ARMmbed/mbed-os-maintainers @ARMmbed/mbed-os-tools please review.

"ld": ["-Wl,--gc-sections", "-Wl,--wrap,main", "-Wl,--wrap,_malloc_r",
"-Wl,--wrap,_free_r", "-Wl,--wrap,_realloc_r", "-Wl,--wrap,_memalign_r",
"-Wl,--wrap,_calloc_r", "-Wl,--wrap,exit", "-Wl,--wrap,atexit",
"-Wl,-n"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I always look for ways to avoid adding another profile when possible. Since this is for debugging, would it be overly intrusive to change the -O0 to -Og in the main debug.json? @deepikabhavnani or @c1728p9 may be able to educate me a bit more.

Copy link
Member

Choose a reason for hiding this comment

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

That would be a dream for many use cases (even if the real deal would be custom settings per file/folder!).
I think @kjbracey-arm must know when it is wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

I dont understand why this is being added. We provide 3 default profiles. If any of these 3 is not sufficient, issue should be raised, not adding a new one.

DTM_StackInit();
////stacktick queueing, the dispatch loop will be initialized after the initialization
////inside main
_eventQueue = mbed_event_queue();
Copy link
Member

Choose a reason for hiding this comment

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

It feels inefficient to pass the default event queue here: If the application doesn't use it like it is the case in all ble examples then you pay for two event queues.
I need some time to think of a way of hooking an undefined recurring event in the Cordio port.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @pan-, rather than in the Cordio it could be better to hook the stack tick inside the dispatch loop of the event queue. I've started studying sleep and deep sleep capabilities of BlueNRG2, discovering that StackTick is also mandatory to get permission from controller to enter sleep mode.

Copy link
Member

Choose a reason for hiding this comment

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

From my understanding BTLE_StackTick has to be called at every system-wakeup to keep up the 500ppm accuracy. There is an integration point in the sleep function.

Going back to attaching it to the default event queue; it is wrong: mbed_event_queue creates a separate thread to that will dispatch events. This is not good resources wise; users must be able to execute their application on a single thread.

BLE API itself doesn't assume the existence of an event queue of any sort: The application must register a callback that will signal the application that BLE must process. That signal can be tied to any type of event queue, a loop, ... It is up to the application.

I'd like to extend the HCI driver as such:

  • Expose a function that can be called by the driver to signal their is work to do.
  • Expose a virtual function that process driver events and that can be overridden by the driver. It would be called whenever the bluetooth subsystem is processed.
// In CordioHCIDriver declaration

// invoke the private virtual function do_process_events();
void process_events(); 

// to call by the driver whenever there is an event. 
void signal_events_to_process();

private: 
// by default event processing does nothing 
virtual void do_process_events() {} 

In the ST driver this can be use as follow:

virtual void initialise() { 
	/* Stack Initialization */
     	DTM_StackInit();

        // setup local Ticker for BTLE_StackTick
        _ticker.attach(mbed::callback(this, signal_events_to_process), 10000 /*us*/);
}

virtual void do_process_events() { 
    BTLE_StackTick();
}

@donatieng @paul-szczepanek-arm What do you think ?

Copy link
Member

@paul-szczepanek-arm paul-szczepanek-arm Feb 1, 2019

Choose a reason for hiding this comment

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

Sounds reasonable, don't see why that precludes the option of automatic handling done on a passed in queue.

Copy link
Author

Choose a reason for hiding this comment

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

@pan- BTLE_StackTick performs the whole link layer and radio processing, it shall be called repeatedly to ensure the correct BLE controller operation (among the others to keep the 500 ppm accuracy, but it is not sufficient to call this signal only once after wakeup). Your proposed approach could be, in my opinion, ideal to ensure the correct processing and at the same time hide this mechanism to the application, with the actual BlueNRG2 controller library implementation.

Concerning the event queue, I had to modify the HRM example to use also there the pointed by the return pointer of mbed_event_queue, by changing with

static EventQueue* event_queue = mbed_event_queue();

@deepikabhavnani
Copy link

@ntoni92 - New target is adding libraries targets/TARGET_STMBLUE/cryptolib/libcrypto.a and targets/TARGET_STMBLUE/Bluetooth_LE/library/libbluenrg1_stack.a, please share more details on the need and compatibility with ARMC6 and IAR8.

CC @SenRamakri @0xc0170

@cmonr
Copy link
Contributor

cmonr commented Jan 31, 2019

@ntoni92 Please take a look at the Travis CI failures.

"STEVAL_IDB008V2": {
"inherits": ["BLUENRG2"],
"extra_labels_add": ["BLUENRG2"],
"release_versions": ["5"],
Copy link
Contributor

Choose a reason for hiding this comment

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

There's mention about size limitation (withour RTOS ), but adding here version 5? Can you elaborate please

Copy link
Author

Choose a reason for hiding this comment

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

@cmonr thank you, it seems that I've violated something about the hierarchy in the target.json file. I'll fix and update on my next commit.

@0xc0170 about the size limitation, it is not feasible to use the Mbed OS 5 RTOS feature, but from what I've understood, since version 5.6 event queue feature (and thus code examples based on it) is available in non-RTOS solutions too.

Together with @avilei and @donatieng decided to develop the non-RTOS solution on Mbed 5.

@@ -0,0 +1,336 @@
/*******************************************************************************
Copy link
Contributor

Choose a reason for hiding this comment

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

.txt file? Is this used anywhere? there are few .txt files (linker scripts)

Copy link
Author

Choose a reason for hiding this comment

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

@0xc0170 I apologize, it's a commit error. I was using it while refactoring the correct linker script file. I'll eliminate on the next commit.

@@ -0,0 +1,34 @@
#include "stack_user_cfg.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

missing license header file, please add (including SPDX identifier)


#ifdef CONTROLLER_DATA_LENGTH_EXTENSION_ENABLED
#if (CONTROLLER_DATA_LENGTH_EXTENSION_ENABLED == 0)

Copy link
Contributor

Choose a reason for hiding this comment

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

Comment for library/libbluenrg1_stack.a - needs to include readme and license (PBL), please add

Copy link
Author

Choose a reason for hiding this comment

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

@0xc0170 Concerning library/libbluenrg1_stack.a, it implements what is declared and documented in inc/*. These are the official library and official documentation from Stmicroelectronics BlueNRG DK version 3.0.0.
About licensing, I should ask my tutor @avilei since I don't know anything about this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ntoni92
Copy link
Author

ntoni92 commented Mar 11, 2019

Note, #9571 - might have implications for new targets. Please review (there's design document, and targets updated, if any questions, let us know). In any case, please reply to this comment if this change is already done or when completed.

Hi @0xc0170 and @deepikabhavnani,
I've added license information and updated to the suggested memory model.
In addition, I've added support to ARMc6 toolchain. It compiles successfully, but I'm facing some issues with the project exporter and mbed CLI:

  • mbed compile from CLI does not link the static library libbluenrg1_stack.a and there is a fail (undefined symbols);
  • mbed export on uvision6 works, but only after a manually static library insertion (it should be done automatically in my opinion) and remove and re-add BlueNRG-2 CMSIS pack, otherwise there are some errors.

Please review.

Many thanks.

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 11, 2019

@0xc0170 With respect to the static library (libbluenrg1_stack.a), is it feasible to publish it with its original license (SLA0068 from STMicroelectronics), instead of PBL?

PBL as licensing guide suggests. SLA0068 is not permissive license to my understanding.

Regarding an example from Nordic license: what was done was done, we can't change that at the moment. Our license guide is the one to be followed.

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 11, 2019

mbed compile from CLI does not link the static library libbluenrg1_stack.a and there is a fail (undefined symbols);

.ar for armcc

@adbridge
Copy link
Contributor

@ntoni92 is this ready for re-review ?

@cmonr
Copy link
Contributor

cmonr commented Mar 27, 2019

@adbridge I think we're waiting on a resolution to the license.

@ntoni92
Copy link
Author

ntoni92 commented Mar 28, 2019

Hi @adbridge @cmonr, exactly! My supervisor is still waiting on a resolution from ST legal office about the PBL-SLA compatibility issue. Concerning the porting, I've completed the SPI driver, I'll merge soon. BTLE_StackTick (and correlated LL synchronization) is still pending (a callback registration is required at user level in the current BLE examples). In any case, I'm leaving the project from next week, for any further detail about licensing please refer to my supervisor @avilei.

@pan-
Copy link
Member

pan- commented Mar 28, 2019

@paul-szczepanek-arm We should run test on these.

@adbridge
Copy link
Contributor

If there are no further updates on this in the next few days I am proposing to close this until such time as updates are available.

@adbridge
Copy link
Contributor

adbridge commented May 1, 2019

@ntoni92 please re-open this PR once there are updates available.

@adbridge adbridge closed this May 1, 2019
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.

9 participants