Various warning and bug fixes

CMakeLists.txt

@@ -35,8 +35,7 @@ FetchContent_Declare(ArduinoJson
 )
 FetchContent_MakeAvailable(ArduinoJson)
 
-
-add_executable(pico-asha
+set(ASHA_SRC
   src/main.cpp
   src/runtime_settings.cpp
   src/asha_audio.cpp
@@ -51,6 +50,10 @@ add_executable(pico-asha
   lib/g722/g722_encode.cc
 )
 
+set_source_files_properties(${ASHA_SRC} PROPERTIES COMPILE_OPTIONS "-Wall;-Wextra;-Werror")
+
+add_executable(pico-asha ${ASHA_SRC})
+
 target_include_directories(pico-asha PUBLIC src/ lib/)
 
 option(ENABLE_BLE_AD_DUMP "Enable dumping of advertisement packet data" OFF)

lib/g722/g722_encode.cc

@@ -164,7 +164,7 @@ static void block4(g722_band_t *band, int d)
 /*- End of function --------------------------------------------------------*/
 
 g722_encode_state_t *g722_encode_init(g722_encode_state_t *s,
-                                             unsigned int rate, int options)
+                                             unsigned int rate, [[maybe_unused]] int options)
 {
     if (s == NULL)
     {

src/asha_audio.hpp

@@ -3,7 +3,7 @@
 #include <cstdint>
 #include <memory>
 
-#include "pico/async_context.h"
+#include <pico/async_context.h>
 
 #include "g722/g722_enc_dec.h"
 #include "PseudoAtomic/RP2040Atomic.hpp"

src/asha_bt.cpp

@@ -1,12 +1,12 @@
-#include "stdio.h"
-#include "pico/cyw43_arch.h"
-#include "pico/time.h"
-#include "hardware/watchdog.h"
+#include <stdio.h>
+#include <pico/cyw43_arch.h>
+#include <pico/time.h>
+#include <hardware/watchdog.h>
 
-#include "pico/stdio_usb.h"
-#include "pico/stdio_uart.h"
+#include <pico/stdio_usb.h>
+#include <pico/stdio_uart.h>
 
-#include "etl/string.h"
+#include <etl/string.h>
 
 #include <ArduinoJson.h>
 
@@ -52,7 +52,6 @@ static void handle_stdin_line_worker(async_context_t *context, async_when_pendin
 static void set_data_length();
 static void discover_services();
 static void finalise_curr_discovery();
-static bool device_db_empty();
 static void delete_paired_devices();
 static void delete_paired_device(const bd_addr_t addr);
 
@@ -112,7 +111,6 @@ AdvertisingReport::AdvertisingReport(uint8_t* packet, bool extended)
 void AdvertisingReport::check_if_ha(uint8_t length, const uint8_t * data)
 {
     ad_context_t context;
-    bd_addr_t address;
     for (ad_iterator_init(&context, length, data) ; ad_iterator_has_more(&context) ; ad_iterator_next(&context)) {
         uint8_t data_type    = ad_iterator_get_data_type(&context);
         uint8_t size         = ad_iterator_get_data_len(&context);
@@ -157,7 +155,8 @@ void ScanResult::reset()
     services_it = services.end();
 }
 
-static void handle_bt_audio_pending_worker(async_context_t *context, async_when_pending_worker_t *worker)
+static void handle_bt_audio_pending_worker([[maybe_unused]] async_context_t *context, 
+                                           [[maybe_unused]] async_when_pending_worker_t *worker)
 {
     // No need to do anything if no hearing aids are connected!
     using enum HA::State;
@@ -192,7 +191,8 @@ static void handle_bt_audio_pending_worker(async_context_t *context, async_when_
     }
 }
 
-static void handle_stdin_line_worker(async_context_t *context, async_when_pending_worker_t *worker)
+static void handle_stdin_line_worker([[maybe_unused]] async_context_t *context, 
+                                     [[maybe_unused]] async_when_pending_worker_t *worker)
 {
     JsonDocument cmd_doc;
     JsonDocument resp_doc;
@@ -404,7 +404,10 @@ static void discover_services()
     }
 }
 
-static void hci_event_handler(uint8_t packet_type, uint16_t channel, uint8_t *packet, uint16_t size)
+static void hci_event_handler(uint8_t packet_type, 
+                             [[maybe_unused]] uint16_t channel, 
+                             uint8_t *packet, 
+                             [[maybe_unused]] uint16_t size)
 {
     ASHA_ASSERT_PACKET_TYPE(HCI_EVENT_PACKET);
     bd_addr_t local_addr;
@@ -433,7 +436,7 @@ static void hci_event_handler(uint8_t packet_type, uint16_t channel, uint8_t *pa
     /* Every time a GAP advertising report is received, handle it here */
     case GAP_EVENT_EXTENDED_ADVERTISING_REPORT:
         LOG_SCAN("Got extended advertising report.");
-        // Fallthrough
+        [[fallthrough]];
     case GAP_EVENT_ADVERTISING_REPORT:
         if (scan_state != ScanState::Scan) return;
         if (hci_ev_type == GAP_EVENT_EXTENDED_ADVERTISING_REPORT) {
@@ -468,7 +471,10 @@ static void hci_event_handler(uint8_t packet_type, uint16_t channel, uint8_t *pa
             sm_request_pairing(curr_scan.ha.conn_handle);
             break;
         }
+        default:
+            break;
         }
+        break;
     case HCI_EVENT_LE_META:
         switch(hci_event_le_meta_get_subevent_code(packet)) {
         case HCI_SUBEVENT_LE_DATA_LENGTH_CHANGE:
@@ -518,7 +524,10 @@ static void hci_event_handler(uint8_t packet_type, uint16_t channel, uint8_t *pa
 }
 
 /* Handler for security manager events */
-static void sm_event_handler (uint8_t packet_type, uint16_t channel, uint8_t *packet, uint16_t size) {
+static void sm_event_handler (uint8_t packet_type, 
+                              [[maybe_unused]] uint16_t channel, 
+                              uint8_t *packet, 
+                              [[maybe_unused]] uint16_t size) {
     ASHA_ASSERT_PACKET_TYPE(HCI_EVENT_PACKET);
     auto ev_type = hci_event_packet_get_type(packet);
     if (scan_state == ScanState::IdentityResolving) {
@@ -643,7 +652,10 @@ static void sm_event_handler (uint8_t packet_type, uint16_t channel, uint8_t *pa
     }
 }
 
-static void l2cap_cbm_event_handler (uint8_t packet_type, uint16_t channel, uint8_t *packet, uint16_t size)
+static void l2cap_cbm_event_handler (uint8_t packet_type, 
+                                     [[maybe_unused]] uint16_t channel, 
+                                     uint8_t *packet, 
+                                     [[maybe_unused]] uint16_t size)
 {
     if (packet_type != HCI_EVENT_PACKET) return;
     switch (hci_event_packet_get_type(packet)) {
@@ -672,13 +684,26 @@ static void l2cap_cbm_event_handler (uint8_t packet_type, uint16_t channel, uint
         LOG_ERROR(msg ": 0x%02x", static_cast<unsigned int>(res)); \
         scan_state = ScanState::Disconnecting; \
         gap_disconnect(curr_scan.ha.conn_handle); \
+        return; \
     }}
 
+#define GATT_COMPLETE_ASSERT(pkt, msg) { \
+    if (gatt_event_query_complete_get_att_status((pkt)) != ATT_ERROR_SUCCESS) { \
+        LOG_ERROR("%s: 0x%02x", (msg), static_cast<unsigned int>(gatt_event_query_complete_get_att_status((pkt)))); \
+        scan_state = ScanState::Disconnecting; \
+        gap_disconnect(curr_scan.ha.conn_handle); \
+        return; \
+    } \
+}
+
 /* Handler for reading GATT service and characteristic values
    and subscribing to the AudioStatusPoint characteristic notification 
    during the connection process.
    This function can be read in sequence for the order of events */
-static void scan_gatt_event_handler (uint8_t packet_type, uint16_t channel, uint8_t *packet, uint16_t size)
+static void scan_gatt_event_handler ([[maybe_unused]] uint8_t packet_type, 
+                                     [[maybe_unused]] uint16_t channel, 
+                                     uint8_t *packet, 
+                                     [[maybe_unused]] uint16_t size)
 {
     switch (scan_state) {
     case ScanState::ServiceDiscovery:
@@ -706,6 +731,7 @@ static void scan_gatt_event_handler (uint8_t packet_type, uint16_t channel, uint
             break;
         case GATT_EVENT_QUERY_COMPLETE:
         {
+            GATT_COMPLETE_ASSERT(packet, "ATT error discovering services");
             // Older hearing aids may support MFI but not ASHA
             if (!curr_scan.service_found || curr_scan.services.empty()) {
                 LOG_INFO("ASHA service not found. Continuing scanning");
@@ -757,6 +783,7 @@ static void scan_gatt_event_handler (uint8_t packet_type, uint16_t channel, uint
             //          characteristic.start_handle, characteristic.value_handle, characteristic.end_handle);
             break;
         case GATT_EVENT_QUERY_COMPLETE:
+            GATT_COMPLETE_ASSERT(packet, "ATT error discovering characteristics");
             LOG_INFO("Characteristic discovery complete");
             curr_scan.services_it++;
             if (curr_scan.services_it != curr_scan.services.end()) {
@@ -785,14 +812,7 @@ static void scan_gatt_event_handler (uint8_t packet_type, uint16_t channel, uint
         switch (hci_event_packet_get_type(packet)) {
         case GATT_EVENT_QUERY_COMPLETE:
         {
-            auto att_res = gatt_event_query_complete_get_att_status(packet);
-            if (att_res != ATT_ERROR_SUCCESS) {
-                LOG_ERROR("Subscribing to GATT Service changed indication failed: 0x%02x", static_cast<unsigned int>(att_res));
-                scan_state = ScanState::Disconnecting;
-                gap_disconnect(curr_scan.ha.conn_handle);
-                return;
-            }
-
+            GATT_COMPLETE_ASSERT(packet, "ATT error service changed indication");
             LOG_INFO("Subscribed to GATT Service changed indication");
             // Start reading the Device name characteristic
             scan_state = ScanState::ReadDeviceName;
@@ -819,6 +839,7 @@ static void scan_gatt_event_handler (uint8_t packet_type, uint16_t channel, uint
             break;
         }
         case GATT_EVENT_QUERY_COMPLETE:
+            GATT_COMPLETE_ASSERT(packet, "ATT error reading device name");
             LOG_INFO("Completed value read of Device Name");
             // Start reading the Read Only Properties characteristic
             scan_state = ScanState::ReadROP;
@@ -840,6 +861,7 @@ static void scan_gatt_event_handler (uint8_t packet_type, uint16_t channel, uint
             //curr_scan.device.read_only_props.dump_values();
             break;
         case GATT_EVENT_QUERY_COMPLETE:
+            GATT_COMPLETE_ASSERT(packet, "ATT error reading ROP");
             LOG_INFO("Completed value read of ReadOnlyProperties");
             /* Next get the PSM value */
             scan_state = ScanState::ReadPSM;
@@ -861,6 +883,7 @@ static void scan_gatt_event_handler (uint8_t packet_type, uint16_t channel, uint
             LOG_INFO("PSM: %d", static_cast<int>(curr_scan.ha.psm));
             break;
         case GATT_EVENT_QUERY_COMPLETE:
+            GATT_COMPLETE_ASSERT(packet, "ATT error reading PSM");
             LOG_INFO("Completed value read of PSM");
             curr_scan.ha.rop.print_values();
             scan_state = ScanState::Finalizing;
@@ -869,10 +892,15 @@ static void scan_gatt_event_handler (uint8_t packet_type, uint16_t channel, uint
         }
         break;
     }
+    default:
+        break;
     }
 }
 
-static void connected_gatt_event_handler(uint8_t packet_type, uint16_t channel, uint8_t *packet, uint16_t size)
+static void connected_gatt_event_handler([[maybe_unused]] uint8_t packet_type, 
+                                         [[maybe_unused]] uint16_t channel, 
+                                         uint8_t *packet, 
+                                         [[maybe_unused]] uint16_t size)
 {
     switch (hci_event_packet_get_type(packet)) {
     case GATT_EVENT_NOTIFICATION:
@@ -944,21 +972,6 @@ static void finalise_curr_discovery()
     }
 }
 
-static bool device_db_empty()
-{
-    int addr_type;
-    bd_addr_t addr;
-    sm_key_t sm_key;
-    int max_db = le_device_db_max_count();
-    for (int i = 0; i < max_db; ++i) {
-        le_device_db_info(i, &addr_type, addr, sm_key);
-        if (addr_type != BD_ADDR_TYPE_UNKNOWN) {
-            return false;
-        }
-    }
-    return true;
-}
-
 static void delete_paired_devices()
 {
     LOG_INFO("Removing paired devices");

src/asha_bt.hpp

@@ -1,8 +1,8 @@
 #pragma once
 
 #include <cstdint>
-#include "etl/vector.h"
-#include "btstack.h"
+#include <etl/vector.h>
+#include <btstack.h>
 
 #include "hearing_aid.hpp"
 

src/asha_led.cpp

@@ -1,14 +1,15 @@
 #include <bitset>
+#include <pico/cyw43_arch.h>
 
 #include "asha_led.hpp"
-#include "pico/cyw43_arch.h"
 
 namespace asha
 {
 
 LEDManager::LEDManager()
 {
     led_worker.user_data = this;
+    led_worker.do_work = &LEDManager::handle_led_work;
 }
 
 void LEDManager::set_led(LEDManager::State led_state)
@@ -34,9 +35,9 @@ void LEDManager::handle_led_work(async_context_t* ctx, async_at_time_worker_t *w
     ++p.pos;
     if (p.pos >= p.len) {
         p.pos = 0;
-        async_context_add_at_time_worker_in_ms(this_->led_ctx, &this_->led_worker, p.interval_ms + p.delay_ms);
+        async_context_add_at_time_worker_in_ms(ctx, worker, p.interval_ms + p.delay_ms);
     } else {
-        async_context_add_at_time_worker_in_ms(this_->led_ctx, &this_->led_worker, p.interval_ms);
+        async_context_add_at_time_worker_in_ms(ctx, worker, p.interval_ms);
     }
 }
 

src/asha_led.hpp

@@ -2,7 +2,7 @@
 
 #include <cstdint>
 
-#include "pico/async_context.h"
+#include <pico/async_context.h>
 
 namespace asha
 {
@@ -39,7 +39,7 @@ class LEDManager
     void disable_curr_led_pattern();
     static void handle_led_work(async_context_t* ctx, async_at_time_worker_t *worker);
     async_context_t *led_ctx = nullptr;
-    async_at_time_worker_t led_worker = {.do_work = &LEDManager::handle_led_work};
+    async_at_time_worker_t led_worker = {};
     Pattern curr_pattern = {};
 };
 

src/asha_logging.cpp

@@ -1,11 +1,11 @@
-#include "pico/stdio_usb.h"
+#include <pico/stdio_usb.h>
 
 #include "asha_logging.h"
 
 namespace asha
 {
 
-void handle_logging_pending_worker(async_context_t *context, async_when_pending_worker_t *worker)
+void handle_logging_pending_worker([[maybe_unused]] async_context_t *context, [[maybe_unused]] async_when_pending_worker_t *worker)
 {
     while(!log_buffer.empty() && stdio_usb_connected()) {
         printf("%s", log_buffer.front().c_str());

src/asha_logging.h

@@ -1,15 +1,15 @@
 #pragma once
 
-#include "stdio.h"
+#include <stdio.h>
 
-#include "etl/circular_buffer.h"
-#include "etl/string.h"
+#include <etl/circular_buffer.h>
+#include <etl/string.h>
 
-#include "pico/time.h"
-#include "pico/async_context.h"
+#include <pico/time.h>
+#include <pico/async_context.h>
 
-#include "hci_dump_embedded_stdout.h"
-#include "btstack_debug.h"
+#include <hci_dump_embedded_stdout.h>
+#include <btstack_debug.h>
 #include "runtime_settings.hpp"
 
 namespace asha
@@ -34,12 +34,14 @@ static void asha_log(enum LogLevel level, const char* fmt, Arg...args)
     } else {
         etl::string<log_line_len> line = {};
         int len = snprintf(line.data(), line.capacity(), fmt, log_level_to_str(level), to_ms_since_boot(get_absolute_time()), args...);
-        line.uninitialized_resize(len <= log_line_len ? len : log_line_len);
-        log_buffer.push(line);
-        if (logging_ctx) {
-            async_context_set_work_pending(logging_ctx, &logging_pending_worker);
+        if (len >= 0) {
+            line.uninitialized_resize((size_t)len <= log_line_len ? (size_t)len : log_line_len);
+            log_buffer.push(line);
+            if (logging_ctx) {
+                async_context_set_work_pending(logging_ctx, &logging_pending_worker);
+            }
+            //printf(fmt, log_level_to_str(level), to_ms_since_boot(get_absolute_time()), args...);
         }
-        //printf(fmt, log_level_to_str(level), to_ms_since_boot(get_absolute_time()), args...);
     }
 }
 

src/asha_unique_id.hpp

@@ -1,6 +1,6 @@
 #pragma once
 
-#include "pico/unique_id.h"
+#include <pico/unique_id.h>
 
 namespace asha
 {