Apple Notification Center Service - hard coded max attribute length of 32?

[denis]

nrf_ble_ancs_c.h has hardcoded max attribute length of 32 (BLE_ANCS_ATTR_DATA_MAX).

1) BLE_ANCS_ATTR_DATA_MAX mainly seems to be used to allocate the size of the app_id in ble_ancs_c_evt_t.  So maybe is misnamed?

2) BLE_ANCS_ATTR_DATA_MAX is also used for checking the the length passed to nrf_ble_ancs_c_attr_add.  But that doesn't seem to make sense because the buffers are passed in and can be of any size.  Is this a bug?  I want to be able to pass in a large buffer (say 100 bytes) for the message attribute so I can get more than 32 characters of the notification message attribute.

[denis]

It also looks like the null termination on an attribute writes 1 past the buffer end in ancs_attr_parser.c:

    // We have reached the end of the attribute, or our max allocated internal size.
    // Stop copying data over to our buffer. NUL-terminate at the current index.
    if ( (p_ancs->parse_info.current_attr_index == p_ancs->evt.attr.attr_len) ||
         (p_ancs->parse_info.current_attr_index == p_ancs->parse_info.p_attr_list[p_ancs->evt.attr.attr_id].attr_len - 1))
    {
        if (attr_is_requested(p_ancs, p_ancs->evt.attr))
        {
            p_ancs->evt.attr.p_attr_data[p_ancs->parse_info.current_attr_index] = '\0';
        }

[denis]

One other note:  It looks like in the example nrf_ble_ancs_c_attr_add is called before ble_ancs_c_init.  Seems like it would be safer for ble_ancs_c_init to initialize all the ancs_notif_attr_list values to get = false and then call nrf_ble_ancs_c_attr_add only on the attributes that you want to retrieve.  (As it is now if you comment out any of the nrf_ble_ancs_c_attr_add then you have undefined values, which leads to undefined behavior.)

[Håvard]

Hello,

#define BLE_ANCS_ATTR_DATA_MAX              32  //!< Maximum data length of an iOS notification attribute.

1. The comment for the definition could be more clear. The define is also used for application attributes in addition to notification attributes. The App ID is an attribute, so I think the naming of the definition itself is ok. But that is subjective of course.

2. This define is safe to increase so that you can receive more of the attribute. It should have been placed in sdk_config.h.

nrf_ble_ancs_c_app_attr_add() will return NRF_ERROR_INVALID_LENGTH if you supply a "len" param larger than BLE_ANCS_ATTR_DATA_MAX. This could have been mentioned directly in the API documentation.

[denis]

My point is that nrf_ble_ancs_c_app_attr_add should not check the length against BLE_ANCS_ATTR_DATA_MAX, since there doesn't appear to be such a limitation and the buffer passed in can be of any size.

[Håvard]

Looking at this snippet and the code around it in the c file, I think it terminates after the attribute if there is enough buffer to do so.

If the allocated buffer is 32 bytes, and a 20 byte attribute is received, a nul-terminator is placed on byte 21. If the allocated buffer is 10 bytes, and the message received is 10 bytes, the nul-terminator is placed on byte 10, thereby truncating the data, but at least turning it into a printable string. (replacing the last character to fit into the buffer with a '\0' and then ignoring the rest of the attribute because there is no buffer to store it in.)