My comments inline > On Jun 23, 2016, at 2:35 PM, chris collins <[email protected]> wrote: > > Hi all, > > In reviewing the nimble host API, I thought it would be a good idea to > compare it with another BLE host that is in wide use: the Nordic S130. > I used the S130 API reference > (https://developer.nordicsemi.com/nRF51_SDK/nRF51_SDK_v8.x.x/doc/8.1.0/s130/html/modules.html) > to compile the following list of differences. This list is just based > on my reading of the API reference and looking at some sample Nordic > applications; I may have gotten some details wrong. All corrections and > other feedback is greatly appreciated! > > Thanks, > Chris > > **** Stack-to-application communication: > * S130: > * The stack pushes events onto a queue; the application pulls them > off. > > * Nimble: > * The application configures the stack with callback functions; the > stack executes the callbacks to pass information to the > application. > > * Thoughts: > * I like both approaches. Either approach can be wrapped to behave like > the other if necessary. > > **** GATT service registration: > * S130: > * Service registration is done procedurally via sequential calls to > sd_ble_gatts_service_add(), sd_ble_gatts_characteristic_add(), and > similar. > > * Nimble: > * Data-driven table-based approach. The set of supported attributes > are expressed as a series of tables that resides in the C code. > > * Thoughts: > * I personally prefer nimble's approach, as I like to keep data > separate from code. However, this is just personal preference, > and I don't think there is a "right" answer. I am pretty sure > S130's procedural design would be easier to script against, which > makes me think nimble should support something similar. > Internally, nimble uses something similar to the S130's public > API; these functions could be exposed. > > **** GATT server reads and writes: > * S130: > * Requires each characteristic to be associated with a data buffer > specified by the application during registration. When a peer > performs a GATT operation on one of your characteristics, the > stack manipulates the contents of the buffer accordingly. > > * Nimble: > * The application associates a callback with each characteristic. > Whenever a GATT operation is performed on a characteristic, the > callback gets called. There is no buffer explicitly tied to a > characteristic; it is up to the callback to do the right thing > corresponding to the GATT operation being performed. > > * Thoughts: > * I prefer Nimble's approach, as it provides a more straightforward > means of supporting "dynamic" characteristics. By this, I mean it > is easy for the application to generate a characteristic's value > on demand when read. > I dont like the softdevice in this regard; I prefer the nimble approach.
> **** GATT notifications and indications: > * S130: > * When a notification or indication is sent, the stack automatically > fills the message with the value of the associated characteristic. > It does this by reading the buffer associated with it. > > * Notifications and indications can only be sent if the peer has > subscribed to them. > > * Nimble: > * The application can either manually specify the contents of a > notification (ble_gattc_notify_custom()), or have the stack read > the contents of the associated characteristic > (ble_gattc_notify()). > > * Notifications can be sent to any peer at any time. Subscription > is not necessary. > > * Thoughts: > * I think the ability to send free-form notifications is important. > Notifications are the only way for a server to send unsolicited > data to a client, and giving the application more freedom in this > area opens up the possibility of using bluetooth as a generic > transport layer for application protocols. > I wonder if latter softdevices have added this capability? Does seem like a handy thing to have. > **** GATT client procedures: > * S130: > * Procedures which involve multiple sends (e.g., discovery > procedures, long reads, etc.) require repeated function calls by > the application. The stack informs the application of progress > via an event on the queue; the application continues the procedure > by calling the original function again with an updated set of > arguments. > > * Nimble: > * The stack manages procedure state machines internally. The > application is informed of progress via invocations of callback > functions. The application indicates whether the stack should > proceed with the procedure via the callback return code. > > **** CCCD persistence: > * S130: > * The application implements the persistence mechanism. > > * It is up to the application to populate the stack with CCCD state > after it has read it from flash. When a connection is > established, the application reads CCCD data from flash and passes > it to the stack via a call to sd_ble_gatts_sys_attr_set(). > > * The application decides when to persist CCCD state. The application > requests CCCD state from the stack via a call to > sd_ble_gatts_sys_attr_get(). This is generally done immediately > after a device is disconnected. > > * Nimble: > * The application implements the persistence mechanism. > > * The stack tells the application to persist data via calls to the > store_write_cb callback. > > * The stack requests stored data from the application via calls to > the store_read_cb callback. > > **** Security persistence: > * S130: > * The stack has no knowledge of persistence of security material. > The application specifies security data in stack function calls; > this data has presumably been restored from flash. > > * Nimble: > * The stack uses the same persistence mechanism as with CCCD state: > calls to the store_read_cb and store_write_cb callbacks. The > stack decides when data should be persisted and restored. > > **** Timeouts > * S130: > * Timeouts are 16-bit values expresssed in seconds. > > * Nimble: > * Timeouts are 32-bit values expressed in milliseconds. > Are these for user-defined timeouts? Does the specification define some of these timeouts or is it up to the app? How much space would be saved using 16-bit timeouts? Just curious. > **** Privacy > * S130: > * Application specifies a single own-address-type for all > connections. > > * Nimble: > * Application specifies an own-address-type per connection. > > * Thoughts: > * It isn't clear to me how the S130 generates a resolvable private > address if the application never specifies an identity address to > use. > Does the softdevice allow for any of the own address types to be used? When you say “single own address type” for all connections does this apply to both master and slave? Is the S130 specification only for peripherals or can it be either a central or peripheral? > **** Advertising and response data > * S130: > * Application specifies the raw advertising data and response data > in buffers. > > * Nimble: > * Application specifies values of fields (e.g., name, service > UUIDs); the stack converts these into the raw advertising data. > > * Thoughts: > * Nimble's approach is more user-friendly but also more restrictive. > I think it is good to allow the application to fill the data > buffer itself, and nimble should provide this capability. > I agree; nimble should have an option to specify raw data although using it in a non-standard way seems like it wont be a great idea. > **** Advertising > * S130: > * Accepts the HCI Advertising_Type as a parameter. > > * Nimble: > * Slightly higher level; accepts connectable-mode and > discoverable-mode parameters and converts these into the HCI > Advertising_Type. > > **** Querying stack for connection properties > * S130: > * Several functions. Each function retrieves a different set of > information about the specified connection (e.g., > sd_ble_gap_conn_sec_get() and sd_ble_gap_rssi_get()). > > * Nimble: > * A single function--ble_gap_find_conn()--retrieves all information > about the specified connection. > > **** Connection termination > * S130: > * The termination function (sd_ble_gap_disconnect()) accepts an HCI > reason code as a parameter. > > * Nimble: > * The termination function (ble_gap_terminate()) always temrinates > * with the same reason: remote user terminated connection. > > * Thoughts: > * The S130 gets this right. The application should be able to > specify the reason for the disconnect. > I was going to say the same thing :-) > On Wed, Jun 22, 2016 at 2:42 PM, Christopher Collins > <[email protected]> wrote: >> Hello all, >> >> This email will be long and dry. If you are not interested in the >> nimble host (bluetooth stack), then you might want to skip it. If you >> are a user of the nimble host, the email will be no less dry, but I >> would really appreciate it if you could expend the time to read it when >> you can. All feedback is welcome. >> >> With the core host functionality mostly stable, I think it is time to >> revisit the API. Some crucial functionality is missing from the host >> API; other functionality is available, but the interfaces will cause >> much scratching of application developers' heads. Since Mynewt is >> nearing the end of its beta phase, now is the time to get the API right >> once and for all. In this email I will propose some changes that I wish >> to make to the nimble host API. >> >> As I said, all feedback is welcome, whether it concerns one of the >> proposed changes or not. If there is anything you don't like about the >> nimble host API, please speak up. Let me be clear: no criticism is too >> derisive :). >> >> ****** GATT >> >> **** Characteristic and descriptor access >> >> * Separate "read" and "write" structs in the ble_gatt_access_ctxt. >> Currently, "chr_access" is overloaded so that it applies to both reads >> and writes. I think this is confusing and error-prone. >> >> * For reads, provide a buffer for the application to fill with response >> data if needed. Currently, the application needs to manage its own >> memory for dynamically-generated characteristic values. >> >> * Characteristic callback access function gets executed when a peer >> subscribes to notifications or indications. Currently there is no way >> of knowing when this happens. >> >> * Add "const" to pointers that are meant to be read-only. >> >> The proposed API is below. >> >> /** >> * Context for reads of characteristics and descriptors. An instance of this >> * gets passed to an application callback whenever a local characteristic or >> * descriptor is read. >> */ >> struct ble_gatt_read_ctxt { >> /** >> * (stack --> app) >> * Maximum number of data bytes the stack can send in the read response. >> * This is based on the connection's ATT MTU. >> */ >> uint16_t max_data_len; >> >> /** >> * (stack --> app) >> * A buffer that the app can use to write the characteristic response >> value >> * to. This buffer can hold up to max_data_len bytes. >> */ >> uint8_t *buf; >> >> /** >> * (app --> stack) >> * App points this at the characteristic data to respond with. Initially >> * this points to "buf". >> */ >> const void *data; >> >> /** >> * (app --> stack) >> * App fills this with the number of data bytes contained in >> characteristic >> * response. >> */ >> uint16_t len; >> }; >> >> /** >> * Context for writes of characteristics and descriptors. An instance of this >> * gets passed to an application callback whenever a local characteristic or >> * descriptor is written. >> */ >> struct ble_gatt_write_ctxt { >> /** >> * (stack --> app) >> * The data that the peer is writing to the characteristic. >> */ >> const void *data; >> >> /** >> * (stack --> app) >> * The number of bytes of characteristic data being written. >> */ >> int len; >> }; >> >> union ble_gatt_access_ctxt { >> struct { >> /** >> * Points to the characteristic defintion corresponding to the >> * characteristic being accessed. This is what the app registered at >> * startup. >> */ >> const struct ble_gatt_chr_def *def; >> >> union { >> struct ble_gatt_read_ctxt read; >> struct ble_gatt_write_ctxt write; >> }; >> } chr; >> >> struct { >> /** >> * Points to the descriptor defintion corresponding to the descriptor >> * being accessed. This is what the app registered at startup. >> */ >> const struct ble_gatt_dsc_def *def; >> >> union { >> struct ble_gatt_read_ctxt read; >> struct ble_gatt_write_ctxt write; >> }; >> } dsc; >> }; >> >> **** Characteristic registration >> >> * In the GATT registration context struct (ble_gatt_register_ctxt), >> rename some fields. Sorry, I know renames are annoying, but I think >> the new names are more intuitive. See the proposed >> ble_gatt_register_ctxt struct below. >> >> union ble_gatt_register_ctxt { >> struct { >> uint16_t handle; >> const struct ble_gatt_svc_def *svc_def; >> } svc; >> >> struct { >> uint16_t def_handle; >> uint16_t val_handle; >> const struct ble_gatt_svc_def *svc_def; >> const struct ble_gatt_chr_def *chr_def; >> } chr; >> >> struct { >> uint16_t dsc_handle; >> const struct ble_gatt_dsc_def *dsc_def; >> uint16_t chr_def_handle; >> uint16_t chr_val_handle; >> const struct ble_gatt_chr_def *chr_def; >> } dsc; >> }; >> >> * The characteristic registration context (ble_gatt_register_ctxt.chr) >> now contains a pointer to the parent service definition. This is >> necessary for identifying the characteristic being registered in case >> multiple characteristics have identical UUIDs. >> >> * The descriptor registration context (ble_gatt_register_ctxt.dsc) now >> contains the parent characteristic value handle (in addition to the >> definition handle that it always contained). >> >> * Ability to look up handles of registered services, characteristics, >> and descriptors: >> >> /** All "out" arguments can be null if you don't need the value. */ >> >> int >> ble_gatt_find_svc(const uint8_t *svc_uuid128, >> uint16_t *out_svc_handle); >> >> int >> ble_gatt_find_chr(const uint8_t *svc_uuid128, const uint8_t *chr_uuid128, >> uint16_t *out_svc_handle, >> uint16_t *out_chr_def_handle, >> uint16_t *out_chr_val_handle); >> >> int >> ble_gatt_find_dsc(const uint8_t *svc_uuid128, const uint8_t *chr_uuid128, >> const uint8_t *dsc_uuid128, >> uint16_t *out_svc_handle, >> uint16_t *out_chr_def_handle, >> uint16_t *out_chr_val_handle, >> uint16_t *out_dsc_handle); >> >> >> **** Multi-response procedures >> >> * Indicate that the procedure has successfully completed by passing a >> status of BLE_HS_EDONE to the application callback. Currently, >> successful completion is indicated with a status of 0 and a null >> attribute pointer. >> >> >> ***** GAP >> >> **** Advertising >> >> * Add a duration_ms parameter. 0 means use a suitable default; >> BLE_GAP_FOREVER means never expire. When an advertising procedure >> times out, the application callback is executed with a status of >> BLE_HS_ETIMEOUT. >> >> * Move own_addr_type from params into a separate parameter. The intent >> is that the advertising parameters struct can be reused for subsequent >> advertisement procedures. The properties that are likely to change >> among different advertising procedures (own_addr_type, peer_addr_type, >> and peer_addr) are passed in separately. Maybe this distinction is >> not very useful, and all parameters should just go into the struct... >> >> * Add "high_duty_cycle" field to ble_gap_adv_params. Currently, >> advertising is always low duty cycle! >> >> * Make read-only parameters const. >> >> * Remove support for default parameters when a null ble_gap_adv_params >> pointer is passed. I think there is enough variation among >> applications that the concept of a default set of parameters doesn't >> make much sense. Instead, the application must pass a valid >> ble_gap_adv_params struct to the ble_gap_adv_start() function. The >> fields which may seem obscure to some application developers are >> optional; if you assign zero to them, the stack fills them in for you. >> >> Here is the proposed advertising API: >> >> struct ble_gap_adv_params { >> /*** Mandatory fields. */ >> uint8_t conn_mode; >> uint8_t disc_mode; >> uint8_t adv_filter_policy; >> uint8_t high_duty_cycle:1; >> >> /*** Optional fields; assign 0 to make the stack calculate them. */ >> uint16_t adv_itvl_min; >> uint16_t adv_itvl_max; >> uint8_t adv_channel_map; >> }; >> >> int ble_gap_adv_start(uint8_t own_addr_type, uint8_t peer_addr_type, >> const uint8_t *peer_addr, uint32_t duration_ms, >> const struct ble_gap_adv_params *adv_params, >> ble_gap_event_fn *cb, void *cb_arg); >> >> **** Advertising data >> >> * Allow app to explicitly specify the flags and tx power fields. >> >> Currently: >> >> * Flags: >> The host always inserted the flags field into undirected >> advertisements. >> >> * TX Power Level: >> The application could indicate whether the tx power level field >> should be included in an advertisement, but it could not specify the >> value; the stack always filled in the value. >> >> Proposed change: >> >> * Flags: >> The application has three options concerning the flags field: >> 1. Don't include it in advertisements (!flags_is_present). >> 2. Explicitly specify the value (flags_is_present && flags != 0). >> 3. Let stack calculate the value (flags_is_present && flags == 0). >> >> For option 3, the calculation is delayed until advertising is >> enabled. The delay is necessary because the flags value depends on >> the type of advertising being performed which is not known at this >> time. >> >> Note: The CSS prohibits advertising a flags value of 0, so this >> method of specifying option 2 vs. 3 is sound. >> >> * TX Power Level: >> The application has three options concerning the TX power level >> field: >> 1. Don't include it in advertisements (!tx_pwr_lvl_is_present). >> 2. Explicitly specify the value >> (tx_pwr_lvl_is_present && >> tx_pwr_lvl != BLE_HS_ADV_TX_PWR_LVL_AUTO). >> 3. Let stack calculate the value >> (tx_pwr_lvl_is_present && >> tx_pwr_lvl == BLE_HS_ADV_TX_PWR_LVL_AUTO). >> >> >> **** Connecting >> >> * Rename ble_gap_conn_initiate() to ble_gap_connect(). >> >> * Move own_addr_type from params into a separate parameter. >> >> * Add a duration_ms parameter. 0 means use a suitable default; >> BLE_GAP_FOREVER means never expire. >> >> * Add role and master_clock_accuracy fields to the ble_gap_conn_desc >> struct. >> >> Proposed connect API: >> >> struct ble_gap_conn_params { >> uint16_t scan_itvl; >> uint16_t scan_window; >> uint16_t itvl_min; >> uint16_t itvl_max; >> uint16_t latency; >> uint16_t supervision_timeout; >> uint16_t min_ce_len; >> uint16_t max_ce_len; >> }; >> >> int ble_gap_connect(uint8_t own_addr_type, >> uint8_t peer_addr_type, const uint8_t *peer_addr, >> const struct ble_gap_conn_params *params, >> ble_gap_event_fn *cb, void *cb_arg); >> >> >> **** Discovery >> >> * Add direct_address_type and direct_address fields to the >> ble_gap_disc_desc struct. These fields are only reported when an "LE >> Direct Advertising Report" event has been received from the >> controller. These fields are necessary when you are using privacy; >> you need to know what address type to use if you initiate a connection >> to the advertiser. >> >> What else? >> >> Thanks, >> Chris
