From 04502633931e74da167a28a65c840d32576e972d Mon Sep 17 00:00:00 2001 From: Kim Sekkelund Date: Fri, 13 Sep 2019 14:06:22 +0200 Subject: [PATCH] Bluetooth: Host: Remove printk dependency from settings Some modules use snprintk to format the settings keys. Unfortunately snprintk is tied with printk which is very large for some embedded systems. To be able to have settings enabled without also enabling printk support, change creation of settings key strings to use bin2hex, strlen and strcpy instead. A utility function to make decimal presentation of a byte value is added as u8_to_dec in lib/os/dec.c Add new Kconfig setting BT_SETTINGS_USE_PRINTK Signed-off-by: Kim Sekkelund --- include/sys/util.h | 15 ++++++ lib/os/CMakeLists.txt | 1 + lib/os/dec.c | 35 +++++++++++++ subsys/bluetooth/host/Kconfig | 13 ++++- subsys/bluetooth/host/gatt.c | 8 +-- subsys/bluetooth/host/keys.c | 4 +- subsys/bluetooth/host/settings.c | 75 +++++++++++++++++++--------- tests/lib/sys/util/CMakeLists.txt | 8 +++ tests/lib/sys/util/prj.conf | 1 + tests/lib/sys/util/src/main.c | 81 +++++++++++++++++++++++++++++++ tests/lib/sys/util/testcase.yaml | 3 ++ 11 files changed, 215 insertions(+), 29 deletions(-) create mode 100644 lib/os/dec.c create mode 100644 tests/lib/sys/util/CMakeLists.txt create mode 100644 tests/lib/sys/util/prj.conf create mode 100644 tests/lib/sys/util/src/main.c create mode 100644 tests/lib/sys/util/testcase.yaml diff --git a/include/sys/util.h b/include/sys/util.h index c3655705759..2231931a679 100644 --- a/include/sys/util.h +++ b/include/sys/util.h @@ -170,6 +170,21 @@ size_t bin2hex(const u8_t *buf, size_t buflen, char *hex, size_t hexlen); */ size_t hex2bin(const char *hex, size_t hexlen, u8_t *buf, size_t buflen); +/** + * @brief Convert a u8_t into decimal string representation. + * + * Convert a u8_t value into ASCII decimal string representation. + * The string is terminated if there is enough space in buf. + * + * @param[out] buf Address of where to store the string representation. + * @param[in] buflen Size of the storage area for string representation. + * @param[in] value The value to convert to decimal string + * + * @return The length of the converted string (excluding terminator if + * any), or 0 if an error occurred. + */ +u8_t u8_to_dec(char *buf, u8_t buflen, u8_t value); + #endif /* !_ASMLANGUAGE */ /* KB, MB, GB */ diff --git a/lib/os/CMakeLists.txt b/lib/os/CMakeLists.txt index 1640873f5fc..4c2238d093b 100644 --- a/lib/os/CMakeLists.txt +++ b/lib/os/CMakeLists.txt @@ -7,6 +7,7 @@ zephyr_sources( crc16_sw.c crc8_sw.c crc7_sw.c + dec.c fdtable.c hex.c mempool.c diff --git a/lib/os/dec.c b/lib/os/dec.c new file mode 100644 index 00000000000..7a5724bd4ae --- /dev/null +++ b/lib/os/dec.c @@ -0,0 +1,35 @@ +/* + * Copyright (c) 2019 Oticon A/S + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include + +u8_t u8_to_dec(char *buf, u8_t buflen, u8_t value) +{ + u8_t divisor = 100; + u8_t num_digits = 0; + u8_t digit; + + while (buflen > 0 && divisor > 0) { + digit = value / divisor; + if (digit != 0 || divisor == 1 || num_digits != 0) { + *buf = (char)digit + '0'; + buf++; + buflen--; + num_digits++; + } + + value -= digit * divisor; + divisor /= 10; + } + + if (buflen) { + *buf = '\0'; + } + + return num_digits; +} + diff --git a/subsys/bluetooth/host/Kconfig b/subsys/bluetooth/host/Kconfig index 6724e7d852f..728217b33eb 100644 --- a/subsys/bluetooth/host/Kconfig +++ b/subsys/bluetooth/host/Kconfig @@ -160,7 +160,7 @@ config BT_HOST_CRYPTO config BT_SETTINGS bool "Store Bluetooth state and configuration persistently" - depends on SETTINGS && PRINTK + depends on SETTINGS select MPU_ALLOW_FLASH_WRITE if ARM_MPU help When selected, the Bluetooth stack will take care of storing @@ -186,6 +186,17 @@ config BT_SETTINGS_CCC_STORE_ON_WRITE Choosing this option is safer for battery-powered devices or devices that expect to be reset suddenly. However, it requires additional workqueue stack space. + +config BT_SETTINGS_USE_PRINTK + bool "Use snprintk to encode Bluetooth settings key strings" + depends on SETTINGS && PRINTK + default y + help + When selected, Bluetooth settings will use snprintk to encode + key strings. + When not selected, Bluetooth settings will use a faster builtin + function to encode the key string. The drawback is that if + printk is enabled then the program memory footprint will be larger. endif # BT_SETTINGS config BT_WHITELIST diff --git a/subsys/bluetooth/host/gatt.c b/subsys/bluetooth/host/gatt.c index bad0071acd8..855916f4b96 100644 --- a/subsys/bluetooth/host/gatt.c +++ b/subsys/bluetooth/host/gatt.c @@ -3412,7 +3412,7 @@ static int bt_gatt_store_cf(struct bt_conn *conn) if (conn->id) { char id_str[4]; - snprintk(id_str, sizeof(id_str), "%u", conn->id); + u8_to_dec(id_str, sizeof(id_str), conn->id); bt_settings_encode_key(key, sizeof(key), "cf", &conn->le.dst, id_str); } @@ -3538,7 +3538,7 @@ int bt_gatt_store_ccc(u8_t id, const bt_addr_le_t *addr) if (id) { char id_str[4]; - snprintk(id_str, sizeof(id_str), "%u", id); + u8_to_dec(id_str, sizeof(id_str), id); bt_settings_encode_key(key, sizeof(key), "ccc", (bt_addr_le_t *)addr, id_str); } else { @@ -3609,7 +3609,7 @@ static int bt_gatt_clear_ccc(u8_t id, const bt_addr_le_t *addr) if (id) { char id_str[4]; - snprintk(id_str, sizeof(id_str), "%u", id); + u8_to_dec(id_str, sizeof(id_str), id); bt_settings_encode_key(key, sizeof(key), "ccc", (bt_addr_le_t *)addr, id_str); } else { @@ -3647,7 +3647,7 @@ static int bt_gatt_clear_cf(u8_t id, const bt_addr_le_t *addr) if (id) { char id_str[4]; - snprintk(id_str, sizeof(id_str), "%u", id); + u8_to_dec(id_str, sizeof(id_str), id); bt_settings_encode_key(key, sizeof(key), "cf", (bt_addr_le_t *)addr, id_str); } else { diff --git a/subsys/bluetooth/host/keys.c b/subsys/bluetooth/host/keys.c index f6ceb147d1b..bdd8a0712d1 100644 --- a/subsys/bluetooth/host/keys.c +++ b/subsys/bluetooth/host/keys.c @@ -216,7 +216,7 @@ void bt_keys_clear(struct bt_keys *keys) if (keys->id) { char id[4]; - snprintk(id, sizeof(id), "%u", keys->id); + u8_to_dec(id, sizeof(id), keys->id); bt_settings_encode_key(key, sizeof(key), "keys", &keys->addr, id); } else { @@ -258,7 +258,7 @@ int bt_keys_store(struct bt_keys *keys) if (keys->id) { char id[4]; - snprintk(id, sizeof(id), "%u", keys->id); + u8_to_dec(id, sizeof(id), keys->id); bt_settings_encode_key(key, sizeof(key), "keys", &keys->addr, id); } else { diff --git a/subsys/bluetooth/host/settings.c b/subsys/bluetooth/host/settings.c index fcbf72396b8..58f4a21e733 100644 --- a/subsys/bluetooth/host/settings.c +++ b/subsys/bluetooth/host/settings.c @@ -19,6 +19,7 @@ #include "hci_core.h" #include "settings.h" +#if defined(BT_SETTINGS_USE_PRINTK) void bt_settings_encode_key(char *path, size_t path_size, const char *subsys, bt_addr_le_t *addr, const char *key) { @@ -38,12 +39,59 @@ void bt_settings_encode_key(char *path, size_t path_size, const char *subsys, BT_DBG("Encoded path %s", log_strdup(path)); } +#else +void bt_settings_encode_key(char *path, size_t path_size, const char *subsys, + bt_addr_le_t *addr, const char *key) +{ + size_t len = 3; + + /* Skip if path_size is less than 3; strlen("bt/") */ + if (len < path_size) { + /* Key format: + * "bt///", "/" is optional + */ + strcpy(path, "bt/"); + strncpy(&path[len], subsys, path_size - len); + len = strlen(path); + if (len < path_size) { + path[len] = '/'; + len++; + } + + for (s8_t i = 5; i >= 0 && len < path_size; i--) { + len += bin2hex(&addr->a.val[i], 1, &path[len], + path_size - len); + } + + if (len < path_size) { + /* Type can be either BT_ADDR_LE_PUBLIC or + * BT_ADDR_LE_RANDOM (value 0 or 1) + */ + path[len] = '0' + addr->type; + len++; + } + + if (key && len < path_size) { + path[len] = '/'; + len++; + strncpy(&path[len], key, path_size - len); + len += strlen(&path[len]); + } + + if (len >= path_size) { + /* Truncate string */ + path[path_size - 1] = '\0'; + } + } else if (path_size > 0) { + *path = '\0'; + } + + BT_DBG("Encoded path %s", log_strdup(path)); +} +#endif int bt_settings_decode_key(const char *key, bt_addr_le_t *addr) { - bool high; - int i; - if (settings_name_next(key, NULL) != 13) { return -EINVAL; } @@ -56,25 +104,8 @@ int bt_settings_decode_key(const char *key, bt_addr_le_t *addr) return -EINVAL; } - for (i = 5, high = true; i >= 0; key++) { - u8_t nibble; - - if (*key >= '0' && *key <= '9') { - nibble = *key - '0'; - } else if (*key >= 'a' && *key <= 'f') { - nibble = *key - 'a' + 10; - } else { - return -EINVAL; - } - - if (high) { - addr->a.val[i] = nibble << 4; - high = false; - } else { - addr->a.val[i] |= nibble; - high = true; - i--; - } + for (u8_t i = 0; i < 6; i++) { + hex2bin(&key[i * 2], 2, &addr->a.val[5 - i], 1); } BT_DBG("Decoded %s as %s", log_strdup(key), bt_addr_le_str(addr)); diff --git a/tests/lib/sys/util/CMakeLists.txt b/tests/lib/sys/util/CMakeLists.txt new file mode 100644 index 00000000000..ac5690daa2b --- /dev/null +++ b/tests/lib/sys/util/CMakeLists.txt @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: Apache-2.0 + +cmake_minimum_required(VERSION 3.13.1) +include($ENV{ZEPHYR_BASE}/cmake/app/boilerplate.cmake NO_POLICY_SCOPE) +project(lib_sys_util_tests) + +FILE(GLOB app_sources src/*.c) +target_sources(app PRIVATE ${app_sources}) diff --git a/tests/lib/sys/util/prj.conf b/tests/lib/sys/util/prj.conf new file mode 100644 index 00000000000..9467c292689 --- /dev/null +++ b/tests/lib/sys/util/prj.conf @@ -0,0 +1 @@ +CONFIG_ZTEST=y diff --git a/tests/lib/sys/util/src/main.c b/tests/lib/sys/util/src/main.c new file mode 100644 index 00000000000..276b1eb6b7c --- /dev/null +++ b/tests/lib/sys/util/src/main.c @@ -0,0 +1,81 @@ +/* + * Copyright (c) 2019 Oticon A/S + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include + +/** + * @brief Test of u8_to_dec + * + * This test verifies conversion of various input values. + * + */ +static void test_u8_to_dec(void) +{ + char text[4]; + u8_t len; + + len = u8_to_dec(text, sizeof(text), 0); + zassert_equal(len, 1, "Length of 0 is not 1"); + zassert_equal(strcmp(text, "0"), 0, + "Value=0 is not converted to \"0\""); + + len = u8_to_dec(text, sizeof(text), 1); + zassert_equal(len, 1, "Length of 1 is not 1"); + zassert_equal(strcmp(text, "1"), 0, + "Value=1 is not converted to \"1\""); + + len = u8_to_dec(text, sizeof(text), 11); + zassert_equal(len, 2, "Length of 11 is not 2"); + zassert_equal(strcmp(text, "11"), 0, + "Value=10 is not converted to \"11\""); + + len = u8_to_dec(text, sizeof(text), 100); + zassert_equal(len, 3, "Length of 100 is not 3"); + zassert_equal(strcmp(text, "100"), 0, + "Value=100 is not converted to \"100\""); + + len = u8_to_dec(text, sizeof(text), 101); + zassert_equal(len, 3, "Length of 101 is not 3"); + zassert_equal(strcmp(text, "101"), 0, + "Value=101 is not converted to \"101\""); + + len = u8_to_dec(text, sizeof(text), 255); + zassert_equal(len, 3, "Length of 255 is not 3"); + zassert_equal(strcmp(text, "255"), 0, + "Value=255 is not converted to \"255\""); + + memset(text, 0, sizeof(text)); + len = u8_to_dec(text, 2, 123); + zassert_equal(len, 2, + "Length of converted value using 2 byte buffer isn't 2"); + zassert_equal( + strcmp(text, "12"), 0, + "Value=123 is not converted to \"12\" using 2-byte buffer"); + + memset(text, 0, sizeof(text)); + len = u8_to_dec(text, 1, 123); + zassert_equal(len, 1, + "Length of converted value using 1 byte buffer isn't 1"); + zassert_equal( + strcmp(text, "1"), 0, + "Value=123 is not converted to \"1\" using 1-byte buffer"); + + memset(text, 0, sizeof(text)); + len = u8_to_dec(text, 0, 123); + zassert_equal(len, 0, + "Length of converted value using 0 byte buffer isn't 0"); +} + +void test_main(void) +{ + ztest_test_suite(test_lib_sys_util_tests, + ztest_unit_test(test_u8_to_dec) + ); + + ztest_run_test_suite(test_lib_sys_util_tests); +} diff --git a/tests/lib/sys/util/testcase.yaml b/tests/lib/sys/util/testcase.yaml new file mode 100644 index 00000000000..1a60d0fa5ff --- /dev/null +++ b/tests/lib/sys/util/testcase.yaml @@ -0,0 +1,3 @@ +tests: + libraries.sys.util.dec: + tags: lib_sys_util_tests