HID: remove double buffering from inputs, move common behavior to bas…

…e app

device/src/usb/app_base.cpp

@@ -0,0 +1,40 @@
+#include "app_base.hpp"
+
+void app_base::stop()
+{
+    sending_sem_.release();
+}
+
+void app_base::send(const std::span<const uint8_t> &buffer)
+{
+    if (!active()) {
+        return;
+    }
+    if (!sending_sem_.try_acquire_for(SEMAPHORE_RESET_TIMEOUT)) {
+        return;
+    }
+    std::copy(buffer.begin(), buffer.end(), in_buffer_.data() + sizeof(in_id_));
+    auto result = send_report(in_buffer_);
+    if (result != hid::result::OK) {
+        sending_sem_.release();
+    }
+}
+
+void app_base::in_report_sent(const std::span<const uint8_t> &data)
+{
+    if (data.front() != in_id_) {
+        return;
+    }
+    sending_sem_.release();
+}
+
+void app_base::get_report(hid::report::selector select, const std::span<uint8_t> &buffer)
+{
+    if (select != hid::report::selector(hid::report::type::INPUT, in_id_)) {
+        return;
+    }
+    assert(buffer.size() >= in_buffer_.size());
+
+    memcpy(buffer.data(), in_buffer_.data(), in_buffer_.size());
+    send_report(buffer.subspan(0, in_buffer_.size()));
+}

device/src/usb/app_base.hpp

@@ -0,0 +1,42 @@
+#ifndef __APP_BASE_HEADER__
+#define __APP_BASE_HEADER__
+
+#include "hid/application.hpp"
+#include "hid/rdf/descriptor.hpp"
+#include "hid/report_protocol.hpp"
+#include "port/zephyr/semaphore.hpp"
+#include <chrono>
+
+class app_base : public hid::application {
+  public:
+    bool active() const;
+
+  protected:
+    static constexpr std::chrono::milliseconds SEMAPHORE_RESET_TIMEOUT{100};
+
+    template <typename T>
+    static hid::report_protocol rp()
+    {
+        static constexpr const auto rd{T::report_desc()};
+        constexpr hid::report_protocol rp{rd};
+        return rp;
+    }
+    template <typename T, typename TReport>
+    constexpr app_base([[maybe_unused]] T *t, TReport &in_report_buffer)
+        : application(rp<T>()),
+          in_id_(in_report_buffer.ID),
+          in_buffer_(in_report_buffer.data(), sizeof(TReport))
+    {}
+
+    void stop() override;
+    void set_report(hid::report::type type, const std::span<const uint8_t> &data) override {}
+    void get_report(hid::report::selector select, const std::span<uint8_t> &buffer) override;
+    void in_report_sent(const std::span<const uint8_t> &data) override;
+    void send(const std::span<const uint8_t> &buffer);
+
+    const hid::report::id in_id_;
+    const std::span<uint8_t> in_buffer_;
+    os::zephyr::binary_semaphore sending_sem_{1};
+};
+
+#endif // __APP_BASE_HEADER__

device/src/usb/command_app.cpp

@@ -10,12 +10,6 @@ extern "C" bool CommandProtocolTx(const uint8_t *data, size_t size)
 
 extern "C" void CommandProtocolRxHandler(const uint8_t *data, size_t size);
 
-void __attribute__((weak)) CommandProtocolRxHandler(const uint8_t *data, size_t size)
-{
-    printk("CommandProtocolRxHandler: data[0]:%u size:%d\n", data[0], size);
-    CommandProtocolTx(data, size);
-}
-
 command_app &command_app::handle()
 {
     static command_app app{};
@@ -33,12 +27,12 @@ void command_app::set_report(hid::report::type type, const std::span<const uint8
     if (((type != hid::report::type::OUTPUT) || (data.front() != report_ids::OUT_COMMAND))) {
         return;
     }
-    auto &out = *reinterpret_cast<const report_out *>(data.data());
-    CommandProtocolRxHandler(out.payload.data(), data.size() - (report_out::has_id() ? 1 : 0));
-
     // always keep receiving new reports
     out_buffer_.swap_sides();
     receive_report(&out_buffer_[out_buffer_.active_side()]);
+
+    auto &out = *reinterpret_cast<const report_out *>(data.data());
+    CommandProtocolRxHandler(out.payload.data(), data.size() - (report_out::has_id() ? 1 : 0));
 }
 
 void command_app::get_report(hid::report::selector select, const std::span<uint8_t> &buffer)

device/src/usb/controls_app.cpp

@@ -1,14 +1,6 @@
 #include "controls_app.hpp"
 
-extern "C"
-{
-#include "usb/usb.h"
-#include <zephyr/kernel.h>
-}
-
-static K_SEM_DEFINE(reportSending, 1, 1);
-
-controls_app& controls_app::handle()
+controls_app &controls_app::handle()
 {
     static controls_app app{};
     return app;
@@ -17,56 +9,10 @@ controls_app& controls_app::handle()
 void controls_app::start(hid::protocol prot)
 {
     // TODO start handling controls events
-    report_buffer_.reset();
-}
-
-void controls_app::stop()
-{
-    // TODO stop handling controls events
+    report_buffer_ = {};
 }
 
 void controls_app::set_report_state(const controls_report_base<0> &data)
 {
-    k_sem_take(&reportSending, K_MSEC(SEMAPHORE_RESET_TIMEOUT));
-    auto buf_idx = report_buffer_.active_side();
-    auto &report = report_buffer_[buf_idx];
-    report.consumer_codes = data.consumer_codes;
-    report.system_codes = data.system_codes;
-    report.telephony_codes = data.telephony_codes;
-    send_buffer(buf_idx);
-}
-
-void controls_app::send_buffer(uint8_t buf_idx)
-{
-    if (!report_buffer_.differs()) {
-        k_sem_give(&reportSending);
-        return;
-    }
-    if (send_report(&report_buffer_[buf_idx]) == hid::result::OK) {
-        report_buffer_.compare_swap_copy(buf_idx);
-        k_sem_give(&reportSending);
-    }
-}
-
-void controls_app::in_report_sent(const std::span<const uint8_t> &data)
-{
-    if (data.front() != report_ids::IN_CONTROLS) {
-        return;
-    }
-    auto buf_idx = report_buffer_.indexof(data.data());
-    if (!report_buffer_.compare_swap_copy(buf_idx)) {
-        send_buffer(1 - buf_idx);
-    }
-}
-
-void controls_app::get_report(hid::report::selector select, const std::span<uint8_t> &buffer)
-{
-    if (select != controls_report::selector()) {
-        return;
-    }
-    // copy to buffer to avoid overwriting data in transit
-    auto &report = report_buffer_[report_buffer_.inactive_side()];
-    assert(buffer.size() >= sizeof(report));
-    memcpy(buffer.data(), report.data(), sizeof(report));
-    send_report(buffer.subspan(0, sizeof(report)));
+    send({data.data(), sizeof(data)});
 }

device/src/usb/controls_app.hpp

@@ -1,27 +1,22 @@
 #ifndef __CONTROLS_APP_HEADER__
 #define __CONTROLS_APP_HEADER__
 
-#include "double_buffer.hpp"
-#include "hid/application.hpp"
+#include "app_base.hpp"
 #include "hid/page/consumer.hpp"
 #include "hid/page/generic_desktop.hpp"
 #include "hid/page/telephony.hpp"
-#include "hid/rdf/descriptor.hpp"
 #include "hid/report_array.hpp"
-#include "hid/report_protocol.hpp"
 #include "report_ids.h"
 
 using system_code = hid::page::generic_desktop;
 using consumer_code = hid::page::consumer;
 using telephony_code = hid::page::telephony;
 
-class controls_app : public hid::application {
+class controls_app : public app_base {
     static constexpr size_t CONSUMER_CODE_COUNT = 2;
     static constexpr size_t SYSTEM_CODE_COUNT = 2;
     static constexpr size_t TELEPHONY_CODE_COUNT = 2;
 
-    controls_app() : application(report_protocol()) {}
-
   public:
     template <hid::report::id::type REPORT_ID>
     struct controls_report_base : public hid::report::base<hid::report::type::INPUT, REPORT_ID> {
@@ -82,25 +77,12 @@ class controls_app : public hid::application {
     void set_report_state(const controls_report_base<0> &data);
 
   private:
-    static hid::report_protocol report_protocol()
-    {
-        static constexpr const auto rd{report_desc()};
-        constexpr hid::report_protocol rp{rd};
-        return rp;
-    }
+    controls_app() : app_base(this, report_buffer_) {}
 
     void start(hid::protocol prot) override;
-    void stop() override;
-    void set_report(hid::report::type type, const std::span<const uint8_t> &data) override
-    {
-        // no FEATURE or OUTPUT reports
-    }
-    void in_report_sent(const std::span<const uint8_t> &data) override;
-    void get_report(hid::report::selector select, const std::span<uint8_t> &buffer) override;
-    void send_buffer(uint8_t buf_idx);
 
     using controls_report = controls_report_base<report_ids::IN_CONTROLS>;
-    double_buffer<controls_report> report_buffer_{};
+    C2USB_USB_TRANSFER_ALIGN(controls_report, report_buffer_){};
 };
 
 using controls_buffer = controls_app::controls_report_base<0>;

device/src/usb/double_buffer.hpp

@@ -3,23 +3,23 @@
 
 #include <array>
 #include <atomic>
-#include <cstring>
 #include <c2usb.hpp>
+#include <cstring>
 
 template <typename T>
-class double_buffer
-{
-    struct aligned_storage
-    {
+class double_buffer {
+    struct aligned_storage {
         C2USB_USB_TRANSFER_ALIGN(T, m_){};
+        constexpr bool operator==(const aligned_storage &other) const = default;
+        constexpr bool operator!=(const aligned_storage &other) const = default;
     };
 
   public:
     constexpr double_buffer() {}
 
-    T& operator[](uint8_t i) { return buffers_[i].m_; }
+    T &operator[](uint8_t i) { return buffers_[i].m_; }
 
-    const T& operator[](uint8_t i) const { return buffers_[i].m_; }
+    const T &operator[](uint8_t i) const { return buffers_[i].m_; }
 
     uint8_t active_side(std::memory_order mo = std::memory_order::seq_cst) const
     {
@@ -32,7 +32,7 @@ class double_buffer
     }
 
     template <typename TArg>
-    uint8_t indexof(TArg* ptr) const
+    uint8_t indexof(TArg *ptr) const
     {
         auto oneptr = reinterpret_cast<std::uintptr_t>(&buffers_[1]);
         auto dataptr = reinterpret_cast<std::uintptr_t>(ptr);
@@ -48,8 +48,7 @@ class double_buffer
 
     bool compare_swap_copy(uint8_t i, std::memory_order mo = std::memory_order::seq_cst)
     {
-        if (compare_swap(i, mo))
-        {
+        if (compare_swap(i, mo)) {
             buffers_[1 - i] = buffers_[i];
             return true;
         }
@@ -58,7 +57,8 @@ class double_buffer
 
     void reset() { buffers_ = {}; }
 
-    bool differs() const { return std::memcmp(&buffers_[0], &buffers_[1], sizeof(T)) != 0; }
+    // bool differs() const { return std::memcmp(&buffers_[0] != &buffers_[1], sizeof(T)) != 0; }
+    bool differs() const { return buffers_[0] != buffers_[1]; }
 
   private:
     std::array<aligned_storage, 2> buffers_{};

device/src/usb/gamepad_app.cpp

@@ -1,14 +1,6 @@
 #include "gamepad_app.hpp"
 
-extern "C"
-{
-#include "usb/usb.h"
-#include <zephyr/kernel.h>
-}
-
-static K_SEM_DEFINE(reportSending, 1, 1);
-
-gamepad_app& gamepad_app::handle()
+gamepad_app &gamepad_app::handle()
 {
     static gamepad_app app{};
     return app;
@@ -31,7 +23,6 @@ void gamepad_app::stop()
 
 void gamepad_app::set_report_state(const gamepad_report &data)
 {
-    k_sem_take(&reportSending, K_MSEC(SEMAPHORE_RESET_TIMEOUT));
     auto buf_idx = report_buffer_.active_side();
     auto &report = report_buffer_[buf_idx];
     report.buttons = data.buttons;
@@ -45,12 +36,10 @@ void gamepad_app::set_report_state(const gamepad_report &data)
 void gamepad_app::send_buffer(uint8_t buf_idx)
 {
     if (!report_buffer_.differs()) {
-        k_sem_give(&reportSending);
         return;
     }
     if (send_report(&report_buffer_[buf_idx]) == hid::result::OK) {
         report_buffer_.compare_swap_copy(buf_idx);
-        k_sem_give(&reportSending);
     }
 }
 

device/src/usb/hid_battery_app.hpp

@@ -46,6 +46,8 @@ class hid_battery_app : public hid::application {
     {
         uint8_t remaining_capacity : 7;
         bool charging : 1;
+
+        bool operator==(const report &other) const = default;
     };
 
     static hid_battery_app& handle();