Merge pull request #43 from stevenewald/callbacks

Improve callback abstraction

Author: stevenewald

include/drivers/button_driver.hpp

@@ -5,6 +5,7 @@
 #include "drivers/gpio_pin.hpp"
 #include "drivers/gpio_pin_event.hpp"
 #include "microbit_v2.h"
+#include "scheduler/user_callback_storage.hpp"
 
 namespace edge::drivers {
 class ButtonController {
@@ -17,9 +18,8 @@ class ButtonController {
     GPIOPinEvent event_a;
     GPIOPinEvent event_b;
 
-    using SubscriptionArray = etl::array<ProcessCallbackPtr, MAX_PROCESSES>;
-    SubscriptionArray a_subscriptions;
-    SubscriptionArray b_subscriptions;
+    UserCallbackStorage a_subscriptions;
+    UserCallbackStorage b_subscriptions;
 
     ButtonController();
     ~ButtonController() = default;
@@ -35,7 +35,7 @@ class ButtonController {
     bool get_button_pressed(ButtonType button_type);
 
     void subscribe_button_press(
-        ButtonType type, ProcessCallbackPtr callback, uint8_t process_id
+        ButtonType type, ProcessCallbackPtr callback, ProcessId process_id
     );
 };
 } // namespace edge::drivers

include/drivers/driver_commands.hpp

@@ -13,7 +13,7 @@ etl::optional<int> handle_command(DriverCommand type, int arg1, int arg2, int ar
 
 void handle_subscribe(
     DriverSubscribe type, ProcessCallbackPtr callback, int arg1, int arg2,
-    uint8_t process_id
+    ProcessId process_id
 );
 
 } // namespace edge::drivers

include/faults/fault_handler.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "config.hpp"
+#include "scheduler/user_callback_storage.hpp"
 #include "util.hpp"
 
 namespace edge {
@@ -9,7 +10,7 @@ class FaultHandler {
     FaultHandler();
     ~FaultHandler();
 
-    etl::array<etl::optional<ProcessCallbackPtr>, MAX_PROCESSES> error_callbacks;
+    UserCallbackStorage error_callbacks;
 
 public:
     FaultHandler(const FaultHandler&) = delete;
@@ -22,6 +23,6 @@ class FaultHandler {
     // To be called by raw handlers in raw_fault_handling.cpp
     void fault_triggered(FaultType fault_type, uint32_t* stack_ptr);
 
-    void set_fault_callback(uint8_t process_id, ProcessCallbackPtr callback_ptr);
+    void set_fault_callback(ProcessId process_id, ProcessCallbackPtr callback_ptr);
 };
 } // namespace edge

include/ipc/ipc_manager.hpp

@@ -1,12 +1,13 @@
 #pragma once
 
 #include "config.hpp"
+#include "scheduler/user_callback_storage.hpp"
 #include "util.hpp"
 
 namespace edge {
 class IPCManager {
     etl::unordered_map<ProcessName, uint8_t, MAX_PROCESSES> name_to_id;
-    etl::array<ProcessCallbackPtr, MAX_PROCESSES> ipc_communicators{nullptr};
+    UserCallbackStorage ipc_communicators{};
 
     IPCManager() = default;
     ~IPCManager() = default;
@@ -19,7 +20,7 @@ class IPCManager {
     static IPCManager& get();
 
     void register_callback(
-        uint8_t process_id, const ProcessName& process_name, ProcessCallbackPtr callback
+        ProcessId process_id, const ProcessName& process_name, ProcessCallbackPtr callback
     );
     void send_message(const ProcessName& destination_name, int value);
 };

include/scheduler/pending_process_callbacks.hpp

@@ -28,10 +28,10 @@ class PendingProcessCallbacks {
     PendingProcessCallbacks& operator=(PendingProcessCallbacks&&) = delete;
 
     void add_ready_callback(
-        uint8_t process_id, ProcessCallbackPtr callback, int arg1 = 0, int arg2 = 0
+        ProcessId process_id, ProcessCallbackPtr callback, int arg1 = 0, int arg2 = 0
     );
 
-    etl::optional<drivers::subscribe_callback> get_ready_callback(uint8_t process_id);
+    etl::optional<drivers::subscribe_callback> get_ready_callback(ProcessId process_id);
 
     static PendingProcessCallbacks& get();
 };

include/scheduler/user_callback_storage.hpp

@@ -0,0 +1,19 @@
+#pragma once
+
+#include "util.hpp"
+
+namespace edge {
+
+constexpr size_t MAX_CALLBACKS = 128;
+
+class UserCallbackStorage {
+    etl::unordered_map<ProcessId, ProcessCallbackPtr, MAX_CALLBACKS> callbacks_;
+
+public:
+    bool has_callback(ProcessId id);
+
+    void set_callback(ProcessId id, ProcessCallbackPtr ptr);
+
+    void call_callback(ProcessId id, int arg1 = 0, int arg2 = 0);
+};
+} // namespace edge

include/util.hpp

@@ -5,8 +5,13 @@
 namespace edge {
 [[noreturn]] void panic(const char* reason);
 
-using ProcessCallbackPtr = void (*)(int, int);
+// Arg1 and arg2 are optional
+using ProcessCallbackPtr = void (*)(int arg1, int arg2);
+
+using ProcessId = uint8_t;
+
 using ProcessName = etl::string<20>;
+
 enum class FaultType { Usage, Bus, Memory };
 
 struct exception_stack_registers {

src/drivers/button_driver.cpp

@@ -1,7 +1,7 @@
 #include "drivers/button_driver.hpp"
 
 #include "drivers/driver_enums.hpp"
-#include "scheduler/pending_process_callbacks.hpp"
+#include "scheduler/user_callback_storage.hpp"
 
 namespace edge::drivers {
 
@@ -25,14 +25,14 @@ ButtonController::ButtonController() :
 {}
 
 void ButtonController::subscribe_button_press(
-    ButtonType type, ProcessCallbackPtr callback, uint8_t process_id
+    ButtonType type, ProcessCallbackPtr callback, ProcessId process_id
 )
 {
     if (type == ButtonType::A) {
-        a_subscriptions[process_id] = callback;
+        a_subscriptions.set_callback(process_id, callback);
     }
     else if (type == ButtonType::B) {
-        b_subscriptions[process_id] = callback;
+        b_subscriptions.set_callback(process_id, callback);
     }
 }
 
@@ -48,13 +48,12 @@ bool ButtonController::get_button_pressed(ButtonType button_type)
 
 void ButtonController::handle_button_press(ButtonType type, ButtonState state)
 {
-    SubscriptionArray& button_subscriptions =
+    UserCallbackStorage& button_subscriptions =
         type == ButtonType::A ? a_subscriptions : b_subscriptions;
     for (int process_id = 0; process_id < MAX_PROCESSES; process_id++) {
-        if (button_subscriptions[process_id] != nullptr) {
-            PendingProcessCallbacks::get().add_ready_callback(
-                process_id, button_subscriptions[process_id], static_cast<int>(type),
-                static_cast<int>(state)
+        if (button_subscriptions.has_callback(process_id)) {
+            button_subscriptions.call_callback(
+                process_id, static_cast<int>(type), static_cast<int>(state)
             );
         }
     }

src/drivers/driver_commands.cpp

@@ -36,7 +36,7 @@ etl::optional<int> handle_command(DriverCommand type, int arg1, int arg2, int ar
 
 void handle_subscribe(
     DriverSubscribe type, ProcessCallbackPtr callback, int arg1, int arg2,
-    uint8_t process_id
+    ProcessId process_id
 )
 {
     switch (type) {

src/faults/fault_handler.cpp

@@ -1,7 +1,6 @@
 #include "faults/fault_handler.hpp"
 
 #include "nrf52.h"
-#include "scheduler/pending_process_callbacks.hpp"
 #include "scheduler/scheduler.hpp"
 
 #include <stdio.h>
@@ -49,27 +48,18 @@ FaultHandler& FaultHandler::get()
 }
 
 void FaultHandler::set_fault_callback(
-    uint8_t process_id, ProcessCallbackPtr callback_ptr
+    ProcessId process_id, ProcessCallbackPtr callback_ptr
 )
 {
-    if (error_callbacks[process_id]) {
-        printf("Process %d already set callback for fault handling\n", process_id);
-        return;
-    }
-    error_callbacks[process_id] = callback_ptr;
+    error_callbacks.set_callback(process_id, callback_ptr);
 }
 
 void FaultHandler::fault_triggered(FaultType fault_type, uint32_t* stack_ptr)
 {
-    uint8_t current_task = scheduler.get_current_task();
-    if (!error_callbacks[current_task].has_value()) [[unlikely]] {
-        panic("Error handler not set");
-    }
-
     stack_ptr[6] = increment_pc(stack_ptr[6]);
-    PendingProcessCallbacks::get().add_ready_callback(
-        current_task, error_callbacks[current_task].value(),
-        static_cast<int>(fault_type)
+
+    error_callbacks.call_callback(
+        scheduler.get_current_task(), static_cast<int>(fault_type)
     );
 }
 } // namespace edge

src/ipc/ipc_manager.cpp

@@ -1,6 +1,5 @@
 #include "ipc/ipc_manager.hpp"
 
-#include "scheduler/pending_process_callbacks.hpp"
 #include "util.hpp"
 
 #include <stdio.h>
@@ -13,14 +12,10 @@ IPCManager& IPCManager::get()
 }
 
 void IPCManager::register_callback(
-    uint8_t process_id, const ProcessName& new_process_name, ProcessCallbackPtr callback
+    ProcessId process_id, const ProcessName& new_process_name, ProcessCallbackPtr callback
 )
 {
-    if (ipc_communicators[process_id] != nullptr) [[unlikely]] {
-        panic("IPC registered twice for a process");
-    }
-
-    ipc_communicators[process_id] = callback;
+    ipc_communicators.set_callback(process_id, callback);
     name_to_id[new_process_name] = process_id;
 }
 
@@ -32,8 +27,6 @@ void IPCManager::send_message(const ProcessName& destination_name, int value)
     }
 
     auto process_id = name_to_id[destination_name];
-    auto callback = ipc_communicators[process_id];
-    PendingProcessCallbacks::get().add_ready_callback(process_id, callback, value);
-    return;
+    ipc_communicators.call_callback(process_id, value);
 }
 } // namespace edge

src/scheduler/pending_process_callbacks.cpp

@@ -11,7 +11,7 @@ PendingProcessCallbacks& PendingProcessCallbacks::get()
 }
 
 void PendingProcessCallbacks::add_ready_callback(
-    uint8_t process_id, ProcessCallbackPtr callback, int arg1, int arg2
+    ProcessId process_id, ProcessCallbackPtr callback, int arg1, int arg2
 )
 {
     if (ready_callbacks[process_id].size() == MAX_READY_CALLBACKS) {
@@ -22,7 +22,7 @@ void PendingProcessCallbacks::add_ready_callback(
 }
 
 etl::optional<drivers::subscribe_callback>
-PendingProcessCallbacks::get_ready_callback(uint8_t process_id)
+PendingProcessCallbacks::get_ready_callback(ProcessId process_id)
 {
     if (ready_callbacks[process_id].empty()) {
         return etl::nullopt;

src/scheduler/user_callback_storage.cpp

@@ -0,0 +1,28 @@
+#include "scheduler/user_callback_storage.hpp"
+
+#include "scheduler/pending_process_callbacks.hpp"
+
+namespace edge {
+bool UserCallbackStorage::has_callback(ProcessId id)
+{
+    return callbacks_.find(id) != callbacks_.end();
+}
+
+void UserCallbackStorage::set_callback(ProcessId id, ProcessCallbackPtr ptr)
+{
+    if (has_callback(id)) [[unlikely]] {
+        panic("Attempted to set callback when callback already exists");
+    }
+
+    callbacks_[id] = ptr;
+}
+
+void UserCallbackStorage::call_callback(ProcessId id, int arg1, int arg2)
+{
+    if (!has_callback(id)) {
+		panic("Attempted to call callback when it has not been set");
+    }
+
+    PendingProcessCallbacks::get().add_ready_callback(id, callbacks_[id], arg1, arg2);
+}
+} // namespace edge