From 891cf01b87556c4cae3adbb5cd0150d3469b28ad Mon Sep 17 00:00:00 2001 From: Philip Gladstone Date: Sat, 5 Mar 2022 22:25:12 +0000 Subject: [PATCH] Review comments --- components/modules/ble.c | 22 ++++++++++++---------- components/modules/ledc.c | 2 -- docs/modules/ble.md | 4 +++- install.sh | 2 ++ 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/components/modules/ble.c b/components/modules/ble.c index 2d325119..e8551dd8 100644 --- a/components/modules/ble.c +++ b/components/modules/ble.c @@ -34,6 +34,10 @@ #include #define TAG "ble" +#ifndef CONFIG_BT_NIMBLE_ENABLED +#error You must enable NIMBLE if you want the Lua ble module. Hopefully this can be made automatic some day. +#endif + /* BLE */ #include "esp_nimble_hci.h" #include "nimble/nimble_port.h" @@ -196,7 +200,8 @@ free_gatt_svcs(lua_State *L, const struct ble_gatt_svc_def * svcs) { static int lble_access_cb(uint16_t conn_handle, uint16_t attr_handle, struct ble_gatt_access_ctxt *ctxt, void *arg) { - // Actually the only thing we care about is the arg and the ctxt + UNUSED(conn_handle); + UNUSED(attr_handle); size_t task_block_size = sizeof(task_block_t); @@ -220,6 +225,7 @@ lble_access_cb(uint16_t conn_handle, uint16_t attr_handle, struct ble_gatt_acces task_block->length = OS_MBUF_PKTLEN(ctxt->om); uint16_t outlen; if (ble_hs_mbuf_to_flat(ctxt->om, task_block->buffer, task_block->length, &outlen)) { + free(task_block); return BLE_ATT_ERR_UNLIKELY; } } @@ -232,7 +238,7 @@ lble_access_cb(uint16_t conn_handle, uint16_t attr_handle, struct ble_gatt_acces response_message_t message; while (1) { - if (xQueueReceive(response_queue, &message, (TickType_t) (10000/portTICK_PERIOD_MS) ) != pdPASS) { + if (xQueueReceive(response_queue, &message, (TickType_t) (2000/portTICK_PERIOD_MS) ) != pdPASS) { free(task_block); return BLE_ATT_ERR_UNLIKELY; } @@ -270,6 +276,8 @@ lble_task_cb(task_param_t param, task_prio_t prio) { message.errcode = BLE_ATT_ERR_UNLIKELY; lua_State *L = lua_getstate(); + int top = lua_gettop(L); + lua_rawgeti(L, LUA_REGISTRYINDEX, (int) task_block->arg); // Now we have the characteristic table in -1 lua_getfield(L, -1, "type"); @@ -391,12 +399,12 @@ lble_task_cb(task_param_t param, task_prio_t prio) { } else { lua_pop(L, 1); // Throw away the null write pointer } - lua_pop(L, 1); // THrow away the value + lua_pop(L, 1); // Throw away the value message.errcode = 0; } cleanup: - lua_pop(L, 2); + lua_settop(L, top); message.seqno = task_block->seqno; xQueueSend(response_queue, &message, (TickType_t) 0); @@ -563,7 +571,6 @@ gatt_svr_init(lua_State *L) { struct ble_gatt_svc_def *svcs = NULL; lble_build_gatt_svcs(L, &svcs, ¬ify_handles); - //free_gatt_svcs(L, gatt_svr_svcs); gatt_svr_svcs = svcs; rc = ble_gatts_count_cfg(gatt_svr_svcs); @@ -944,11 +951,6 @@ static int lble_notify(lua_State *L) { ble_gatts_chr_updated(notify_handles[handle]); -/* - if (rc) { - return luaL_error(L, "Must supply a valid handle"); - } -*/ return 0; } diff --git a/components/modules/ledc.c b/components/modules/ledc.c index 28289aed..add40b6d 100644 --- a/components/modules/ledc.c +++ b/components/modules/ledc.c @@ -30,8 +30,6 @@ static int lledc_new_channel( lua_State *L ) ledc_timer.timer_num = opt_checkint_range(L, "timer", -1, 0, LEDC_TIMER_MAX-1); - ledc_timer.clk_cfg = LEDC_AUTO_CLK; - /* Setup channel */ ledc_channel_config_t channel_config = { .speed_mode = ledc_timer.speed_mode, diff --git a/docs/modules/ble.md b/docs/modules/ble.md index 4572747d..561cd5f0 100644 --- a/docs/modules/ble.md +++ b/docs/modules/ble.md @@ -120,11 +120,13 @@ The characteristic table contains the following keys: - `uuid` The UUID of the characteristics. This can be either a 16 byte string or a 2 byte string that identifies the particular characteristic. Typically, 2 byte strings are used for well-known characteristics. - `type` This is the optional type of the value. It has the same value as a unpack code in the `struct` module. -- `value` This is the actual value of the characteristic. This will be a string of bytes unless a `type` value is set. +- `value` This is the actual value of the characteristic. This will be a string of bytes (unless `type` is set). - `read` This is a function that will be invoked to read the value (and so does not need the `value` entry). It should return a string of bytes (unless `type` is set). - `write` This is a function that will be invoked to write the value (and so does not need the `value` entry). It is given a string of bytes (unless `type` is set) - `notify` If this attribute is present then notifications are supported on this characteristic. The value of the `notify` attribute is updated to be an integer which is the value to be passed into `ble.notify()` +In the above functions, the value is that passed to/from the write/read functions is of the type specified by the `type` key. If this key is missing, then the default type is a string of bytes. For example, if `type` is `'B'` then the value is an integer (in the range 0 - 255) and the bluetooth client will see a single byte containing that value. + If the `value` key is present, then the characteristic is read/write. However, if one or `read` or `write` is set to `true`, then it restricts access to that mode. The characteristics are treated as read/write unless only one of the `read` or `write` keys is present and the `value` key is not specified. diff --git a/install.sh b/install.sh index c93e5121..cf18df45 100755 --- a/install.sh +++ b/install.sh @@ -1,5 +1,7 @@ #!/bin/bash +set -e + echo "Installing IDF prerequisites..." IDF_DIR=./sdk/esp32-esp-idf