Re: Mynewt for arduino_101 board

[Vipul Rahane]

+1 for new BSP as opposed to #ifdef.

Regards,
Vipul Rahane

> On Apr 18, 2016, at 2:04 PM, p...@wrada.com wrote:
> 
> I concur with what Chris is saying.  This is how we do it for arduino, but
> its not ideal in my opinion.
> 
> It worked OK for arduino zero versus Zero Pro, but for your case we are
> talking about two very different products (arduino and NRF eval board).
> 
> I wish I had a good answer.  One possibility is to pull stuff that is
> common across all NRF51 boards into a separate component.
> 
> 
> 
> On 4/18/16, 1:02 PM, "Christopher Collins"  wrote:
> 
>> Hi Andre,
>> 
>> On Mon, Apr 18, 2016 at 10:55:49AM +0300, Andrei Emeltchenko wrote:
>>> Hi,
>>> 
>>> I am working on arduino_101 development board.
>>> https://www.arduino.cc/en/Main/ArduinoBoard101
>>> 
>>> It has nrf51 BLE chip, Basically the configuration is the same as for
>>> the nrf51dk-16kbram with the only difference:
>>> 
>>> --- a/hw/bsp/nrf51dk-16kbram/src/hal_bsp.c
>>> +++ b/hw/bsp/nrf51dk-16kbram/src/hal_bsp.c
>>> @@ -24,7 +24,7 @@
>>> static const struct nrf51_uart_cfg uart_cfg = {
>>> .suc_pin_tx = 9,
>>> .suc_pin_rx = 11,
>>> -.suc_pin_rts = 8,
>>> +.suc_pin_rts = 12,
>>> .suc_pin_cts = 10
>>> };
>>> 
>>> 
>>> What is the best way of keeping the change? Making special BSP would be
>>> too expensive for this one line change.
>> 
>> This is indeed an annoying situation.  There is a nearly identical
>> dilemma with the Arduino Zero and the Arduino Zero Pro; the only
>> difference among these two boards is a single pin.
>> 
>> The solution for the Arduino Zero is to use a single BSP, but to select
>> the proper pin at compile time via an #ifdef:
>> 
>>   #ifdef ARDUINO_ZERO_PRO
>>ARDUINO_ZERO_D2 = (8),
>>ARDUINO_ZERO_D4 = (14),
>>   #endif
>> 
>>   #ifdef ARDUINO_ZERO
>>ARDUINO_ZERO_D2 = (14),
>>ARDUINO_ZERO_D4 = (8),
>>   #endif
>> 
>> The appropriate preprocessor symbol is defined by the use of a target
>> feature, as described in the arduino zero tutorial:
>> http://mynewt.apache.org/os/tutorials/arduino_zero/
>> 
>> That said, I am not sure this is the right approach.  I will certainly
>> let others weight in, but my feeling is that it is better to just bite
>> the bullet and create a new BSP.  Both solutions pose maintenance
>> headaches, but I think separate BSPs is simpler and more maintainable.
>> An additional benefit of separate BSPs is that it simplifies target
>> creation for the application developer (just specify BSP, rather than
>> BSP plus feature).
>> 
>> Thanks,
>> Chris
> 

Re: Mynewt for arduino_101 board

[p...@wrada.com]

I concur with what Chris is saying.  This is how we do it for arduino, but
its not ideal in my opinion.

It worked OK for arduino zero versus Zero Pro, but for your case we are
talking about two very different products (arduino and NRF eval board).

I wish I had a good answer.  One possibility is to pull stuff that is
common across all NRF51 boards into a separate component.



On 4/18/16, 1:02 PM, "Christopher Collins"  wrote:

>Hi Andre,
>
>On Mon, Apr 18, 2016 at 10:55:49AM +0300, Andrei Emeltchenko wrote:
>> Hi,
>> 
>> I am working on arduino_101 development board.
>> https://www.arduino.cc/en/Main/ArduinoBoard101
>> 
>> It has nrf51 BLE chip, Basically the configuration is the same as for
>> the nrf51dk-16kbram with the only difference:
>> 
>> --- a/hw/bsp/nrf51dk-16kbram/src/hal_bsp.c
>> +++ b/hw/bsp/nrf51dk-16kbram/src/hal_bsp.c
>> @@ -24,7 +24,7 @@
>>  static const struct nrf51_uart_cfg uart_cfg = {
>>  .suc_pin_tx = 9,
>>  .suc_pin_rx = 11,
>> -.suc_pin_rts = 8,
>> +.suc_pin_rts = 12,
>>  .suc_pin_cts = 10
>>  };
>> 
>> 
>> What is the best way of keeping the change? Making special BSP would be
>> too expensive for this one line change.
>
>This is indeed an annoying situation.  There is a nearly identical
>dilemma with the Arduino Zero and the Arduino Zero Pro; the only
>difference among these two boards is a single pin.
>
>The solution for the Arduino Zero is to use a single BSP, but to select
>the proper pin at compile time via an #ifdef:
>
>#ifdef ARDUINO_ZERO_PRO
> ARDUINO_ZERO_D2 = (8),
> ARDUINO_ZERO_D4 = (14),
>#endif
>
>#ifdef ARDUINO_ZERO
> ARDUINO_ZERO_D2 = (14),
> ARDUINO_ZERO_D4 = (8),
>#endif
>
>The appropriate preprocessor symbol is defined by the use of a target
>feature, as described in the arduino zero tutorial:
>http://mynewt.apache.org/os/tutorials/arduino_zero/
>
>That said, I am not sure this is the right approach.  I will certainly
>let others weight in, but my feeling is that it is better to just bite
>the bullet and create a new BSP.  Both solutions pose maintenance
>headaches, but I think separate BSPs is simpler and more maintainable.
>An additional benefit of separate BSPs is that it simplifies target
>creation for the application developer (just specify BSP, rather than
>BSP plus feature).
>
>Thanks,
>Chris

Re: Mynewt for arduino_101 board

[Christopher Collins]

Hi Andre,

On Mon, Apr 18, 2016 at 10:55:49AM +0300, Andrei Emeltchenko wrote:
> Hi,
> 
> I am working on arduino_101 development board.
> https://www.arduino.cc/en/Main/ArduinoBoard101
> 
> It has nrf51 BLE chip, Basically the configuration is the same as for
> the nrf51dk-16kbram with the only difference:
> 
> --- a/hw/bsp/nrf51dk-16kbram/src/hal_bsp.c
> +++ b/hw/bsp/nrf51dk-16kbram/src/hal_bsp.c
> @@ -24,7 +24,7 @@
>  static const struct nrf51_uart_cfg uart_cfg = {
>  .suc_pin_tx = 9,
>  .suc_pin_rx = 11,
> -.suc_pin_rts = 8,
> +.suc_pin_rts = 12,
>  .suc_pin_cts = 10
>  };
> 
> 
> What is the best way of keeping the change? Making special BSP would be
> too expensive for this one line change.

This is indeed an annoying situation.  There is a nearly identical
dilemma with the Arduino Zero and the Arduino Zero Pro; the only
difference among these two boards is a single pin.

The solution for the Arduino Zero is to use a single BSP, but to select
the proper pin at compile time via an #ifdef:

#ifdef ARDUINO_ZERO_PRO
 ARDUINO_ZERO_D2 = (8),
 ARDUINO_ZERO_D4 = (14),
#endif

#ifdef ARDUINO_ZERO
 ARDUINO_ZERO_D2 = (14),
 ARDUINO_ZERO_D4 = (8),
#endif

The appropriate preprocessor symbol is defined by the use of a target
feature, as described in the arduino zero tutorial:
http://mynewt.apache.org/os/tutorials/arduino_zero/

That said, I am not sure this is the right approach.  I will certainly
let others weight in, but my feeling is that it is better to just bite
the bullet and create a new BSP.  Both solutions pose maintenance
headaches, but I think separate BSPs is simpler and more maintainable.
An additional benefit of separate BSPs is that it simplifies target
creation for the application developer (just specify BSP, rather than
BSP plus feature).

Thanks,
Chris

Re: Proposed changes to Nimble host

[Sterling Hughes]

0 - 63: Core event types (TIMER, MQUEUE_DATA, etc.)
64+: Per-task event types.

So, the options for the host package are:
1. Reserve new core event IDs.  This avoids conflicts, but permanently
   uses up a limited resource.
2. Use arbitrary per-task event IDs.  This has the potential for
   conflicts, and doesn't strike me as a particularly good solution.
3. Use a separate host task.  This allows the host use IDs in the per-task
   ID space without the risk of conflict.
4. Leverage existing core events.  This is what I proposed.  It avoids
   conflicts and doesn't require any new event IDs, but it does feel a
   bit hacky to use the TIMER event ID for something that isn't a timer.




What should use these core events?  I think reserving events is fine.

The other option is to reserve a generic "trampoline" event, which 
basically is like a callout, in that you have a pointer to a function 
call and an arg, and everything can repost them to a task.


I think we should have this regardless, but I'm not opposed to burning 
events for something as critical as the networking stack, for example.


Sterling

Re: Proposed changes to Nimble host

[will sanfilippo]

Yeah, I can see why you chose OS_EVENT_TIMER. It is almost like we should 
rename that event type :-) But I agree with everything you say below; creating 
a new event type for this seems wasteful. I am not quite sure what you mean by 
"My concern there is that applications may want to add special handling for 
certain event types…”. Are you referring to the events that a package may 
require of an application?

Anyway, solving this generically is definitely what we need to do.

Will


> On Apr 18, 2016, at 10:06 AM, Christopher Collins  wrote:
> 
> On Mon, Apr 18, 2016 at 09:43:35AM -0700, Christopher Collins wrote:
>> On Mon, Apr 18, 2016 at 09:18:16AM -0700, will sanfilippo wrote:
>>> For #2, my only “concerns” (if you could call them such) are:
>>> * Using OS_EVENT_TIMER as opposed to some other event. Should all
>>> OS_EVENT_TIMER events be caused by a timer? Probably no big deal… What
>>> events are going to be processed here? Do you envision many host
>>> events?
>> 
>> Yes, I agree.  I think a more appropriate event type would be
>> OS_EVENT_CALLBACK or similar.  I am a bit leery about adding a new OS
>> event type for this case, because it would require all applications to
>> handle an extra event type without any practical benefit.  Perhaps
>> mynewt could relieve this burden with an "os_handle_event()" function
>> which processes these generic events.  My concern there is that
>> applications may want to add special handling for certain event types,
>> so they wouldn't want to call the helper function anyway.
>> 
>> The OS events that the host would generate are:
>>* Incoming ACL data packets.
>>* Incoming HCI events.
>>* Expired timers.
> 
> (I meant "process", not "generate"!)
> 
> Oops... I went down a rabbit hole and forgot to address the main point
> :).  What we would *really* want here is something like:
>* BLE_HS_EVENT_ACL_DATA_IN
>* BLE_HS_EVENT_HCI_EVENT_IN
> 
> However, the issue here is that the event type IDs are defined in a
> single "number-space".  If the host package reserves IDs for its own
> events, then no other packages can use those IDs for its own events
> without a conflict.  The 8-bit ID space is divided into two parts:
> 
> 0 - 63: Core event types (TIMER, MQUEUE_DATA, etc.)
> 64+: Per-task event types.
> 
> So, the options for the host package are:
> 1. Reserve new core event IDs.  This avoids conflicts, but permanently
>   uses up a limited resource.
> 2. Use arbitrary per-task event IDs.  This has the potential for
>   conflicts, and doesn't strike me as a particularly good solution.
> 3. Use a separate host task.  This allows the host use IDs in the per-task
>   ID space without the risk of conflict.
> 4. Leverage existing core events.  This is what I proposed.  It avoids
>   conflicts and doesn't require any new event IDs, but it does feel a
>   bit hacky to use the TIMER event ID for something that isn't a timer.
> 
> I think this might be a common problem for other packages in the future.
> I don't think it is that unusual for a package to not create its own
> task, but still have the need to generate OS events.  So perhaps we
> should think about how to solve this general problem.
> 
> Chris

Re: Proposed changes to Nimble host

[Christopher Collins]

On Mon, Apr 18, 2016 at 09:18:16AM -0700, will sanfilippo wrote:
> For #2, my only “concerns” (if you could call them such) are:
> * Using OS_EVENT_TIMER as opposed to some other event. Should all
> OS_EVENT_TIMER events be caused by a timer? Probably no big deal… What
> events are going to be processed here? Do you envision many host
> events?

Yes, I agree.  I think a more appropriate event type would be
OS_EVENT_CALLBACK or similar.  I am a bit leery about adding a new OS
event type for this case, because it would require all applications to
handle an extra event type without any practical benefit.  Perhaps
mynewt could relieve this burden with an "os_handle_event()" function
which processes these generic events.  My concern there is that
applications may want to add special handling for certain event types,
so they wouldn't want to call the helper function anyway.

The OS events that the host would generate are:
* Incoming ACL data packets.
* Incoming HCI events.
* Expired timers.

> * I wonder about the complexity of this from an application developers
> standpoint. Not saying that what you propose would be more or less
> complex; just something we should consider when making these changes.

I think the taskless design reduces complexity for the application
developer.  If there is no host task, the developer can worry less about
task priorities and stack sizes.  

> On a side note (I guess it is related), we should consider how
> applications are going to initialize the host and/or the controller in
> regards to system memory requirements (i.e. mbufs). While our current
> methodology to create a BLE app is not rocket science, I think we
> could make it a bit simpler.

Yes, definitely.  As you say, the setup is not terribly complicated, but
it does involve a fair number of steps, so it will seem complicated to
someone not familiar with Mynewt.

Chris

Re: Proposed changes to Nimble host

[will sanfilippo]

All sounds excellent!

+1 for #1. That only seems like a good thing.

For #2, my only “concerns” (if you could call them such) are:
* Using OS_EVENT_TIMER as opposed to some other event. Should all 
OS_EVENT_TIMER events be caused by a timer? Probably no big deal… What events 
are going to be processed here? Do you envision many host events?
* I wonder about the complexity of this from an application developers 
standpoint. Not saying that what you propose would be more or less complex; 
just something we should consider when making these changes.

On a side note (I guess it is related), we should consider how applications are 
going to initialize the host and/or the controller in regards to system memory 
requirements (i.e. mbufs). While our current methodology to create a BLE app is 
not rocket science, I think we could make it a bit simpler.


> On Apr 17, 2016, at 3:57 PM, Christopher Collins  wrote:
> 
> Hello all,
> 
> The Mynewt BLE stack is called Nimble.  Nimble consists of two packages:
>* Controller (link-layer) [net/nimble/controller]
>* Host (upper layers) [net/nimble/host]
> 
> This email concerns the Nimble host.  
> 
> As I indicated in an email a few weeks ago, the code size of the Nimble
> host had increased beyond what I considered a reasonable level.  When
> built for the ARM cortex-M4, with security enabled and the log level set
> to INFO, the host code size was about 48 kB.  In recent days, I came up
> with a few ideas for reducing the host code size.  As I explored these
> ideas, I realized that they open the door for some major improvements in
> the fundamental design of the host.  Making these changes would
> introduce some backwards-compatibility issues, but I believe it is
> absolutely the right thing to do.  If we do this, it needs to be done
> now while Mynewt is still in its beta phase.  I have convinced myself
> that this is the right way forward; now I would like to see what the
> community thinks.  As always, all feedback is greatly appreciated.
> 
> There are two major changes that I am proposing:
> 
> 1. All HCI command/acknowledgement exchanges are blocking.
> 
> Background: The host and controller communicate with one another via the
> host-controller-interface (HCI) protocol.  The host sends _commands_ to
> the controller; the controller sends _events_ to the host.  Whenever the
> controller receives a command from the host, it immediately responds
> with an acknowledgement event.  In addition, the controller also sends
> unsolicited events to the host to indicate state changes or to request
> information in a subsequent command.
> 
> In the current host, all HCI commands are sent asynchronously
> (non-blocking).  When the host wants to send an HCI command, it
> schedules a transmit operation by putting an OS event on its own event
> queue.  The event points to a callback which does the actual HCI
> transmission.  The callback also configures a second callback to be
> executed when the expected acknowledgement is received from the
> controller.  Each time the host receives an HCI event from the
> controller, an OS event is put on the host's event queue.  Processing of
> this OS event ultimately calls the configured callback (if it is an
> acknowledgement), or a hardcoded callback (if it is an unsolicited HCI
> event).
> 
> This design works, but it introduces a number of problems.  First, it
> requires the host code to maintain some quite complex state machines for
> what seem like simple HCI exchanges.  This FSM machinery translates into
> a lot of extra code.  There is also a lot of ugliness involved in
> canceling scheduled HCI transmits.
> 
> Another complication with non-blocking HCI commands is that they require
> the host to jump through a lot of hoops to provide feedback to the
> application.  Since all the work is done in parallel by the host task,
> the host has to notify the application of failures by executing
> callbacks configured by the application.  I did not want to place any
> restrictions on what the application is allowed to do during these
> callbacks, which means the host has to ensure that it is in a valid
> state whenever a callback gets executed (no mutexes are locked, for
> example).  This requires the code to use a large number of mutexes and
> temporary copies of host data structures, resulting in a lot of
> complicated code.
> 
> Finally, non-blocking HCI operations complicates the API presented to
> the application.  A single return code from a blocking operation is
> easier to manage than a return code plus the possibility of a callback
> being executed sometime in the future from a different task.  A blocking
> operation collapses several failure scenarios into a single function
> return.
> 
> Making HCI command/acknowledgement exchanges blocking addresses all of
> the above issues:
>* FSM machinery goes away; controller response is indicated in the
>  return code of the HCI send function.
>*