Hello all, Yesterday I made a fairly large commit to the net/nimble/host package in the core repository (the host-side of the BLE stack). I wanted to provide the community with an explanation of these changes and related changes planned for the future. This email will probably be long and detailed, so don't feel bad about skimming it for anything that you find interesting. That said, I would love to hear any and all feedback.
First, here is the relevant commit: Repo: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core Branch: develop Commit: 2fef21d6a1f9d3d83b88ecf11b825a6e5a354a47 Motivation for this change: the code size of net/nimble/host had grown quite large over the last few months. During development of the host, most focus was put on correctness and maintainability, while speed and size considerations were placed in the background. Now that the host is reasonably complete, it is a good time to start optimizing, with the hope that the unit tests will protect against any regressions. The commit consists of two major changes: 1. Use packed structs for over-the-air ATT and L2CAP commands. 2. Compile-out most assertions unless BLE_HS_DEBUG is defined. Regarding 1: Prior to this change, the host code was manually marshalling fields between buffers and properly-aligned structs. This had the benefit of being defined behavior according to the C standard, but at the cost of increased code size. By using the gcc __packed__ attribute, the compiler is able to take advantage of the Cortex-M's ability to perform unaligned accesses, resulting in a substantial code savings. Regarding 2: The host code makes liberal use of the assert() macro. Each invocation of the assert() macro was adding a surprising amount of code to the resulting binary. The size increase is not caused by strings being embedded in the binary; the baselibc assert() does not include the expression text in its expansion, and gcc collapses duplicate filename strings. The size increase is just caused by the comparison and branch implicit in the assert macro. The asserts in the host code are plentiful and conservative. They caused an unwelcome increase in code size, but I didn't want to remove them entirely because they are valuable during regression testing. The standard assert() macro compiles to nothing if the NDEBUG symbol is defined, but I wanted more control over which asserts actually get disabled. As a compromise, I added a second type of assertion that only gets compiled in if the BLE_HS_DEBUG symbol is defined. I think the concept of multiple assert levels will be useful to other packages and that it should be added to the core OS. For this reason, I intentionally made the new assert macro names ugly, as I hope to replace them with something that isn't host-specific. Here are the two macros I added: #if BLE_HS_DEBUG #define BLE_HS_DBG_ASSERT(x) assert(x) #define BLE_HS_DBG_ASSERT_EVAL(x) assert(x) #else #define BLE_HS_DBG_ASSERT(x) #define BLE_HS_DBG_ASSERT_EVAL(x) ((void)(x)) #endif BLE_HS_DBG_ASSERT(x) is self-explanatory; it asserts if the necessary debug symbol is defined. BLE_HS_DBG_ASSERT_EVAL(x) is less obvious. If the BLE_HS_DEBUG symbol is defined, this macro has the same effect as the first one. If the symbol is not defined, this macro evaluates its argument and throws away the result. The reason this macro exists is to prevent gcc from raising "variable set but not used" warnings in code like the following: rc = operation_that_should_never_fail(); BLE_HS_DBG_ASSERT_EVAL(rc == SUCCESS); Note that the BLE_HS_DBG_ASSERT_EVAL() macro differs from the standard assert() macro in this respect. The standard assert() macro does not evalute its argument if NDEBUG is defined, which allows you to write code like the following: assert(expensive_sanity_check() == SUCCESS); Future changes: There are a number of future changes planned for further reducing the host code size. * Pack the HCI command and event structs. * Rework (or remove / replace) the generic FSM implementation; the code has outgrown this facility. If you have made this far and have any suggestions of your own, they would be much appreciated, so please feel free to share them. Thanks, Chris