From 678c3a01d3b5ab171a05fe70ef65ee49b2b3ff94 Mon Sep 17 00:00:00 2001
From: Davide Mor <davide.mor@skywarder.eu>
Date: Thu, 13 Jul 2023 09:16:29 +0200
Subject: [PATCH] [sx1278] Switched implementation to timed waits

---
 src/shared/radio/SX1278/SX1278Common.cpp | 33 ++++++++++++++----------
 src/shared/radio/SX1278/SX1278Common.h   |  5 +++-
 src/tests/radio/sx1278/sx1278-init.h     | 24 ++++++++---------
 3 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/src/shared/radio/SX1278/SX1278Common.cpp b/src/shared/radio/SX1278/SX1278Common.cpp
index cdd167504..51f97052c 100644
--- a/src/shared/radio/SX1278/SX1278Common.cpp
+++ b/src/shared/radio/SX1278/SX1278Common.cpp
@@ -63,9 +63,8 @@ void SX1278Common::setDefaultMode(Mode mode, DioMapping mapping,
                                   InterruptTrigger dio1_trigger,
                                   bool tx_frontend, bool rx_frontend)
 {
-    mutex.lock();
+    miosix::Lock<miosix::FastMutex> lock(mutex);
     enterMode(mode, mapping, dio1_trigger, tx_frontend, rx_frontend);
-    mutex.unlock();
 }
 
 ISX1278::IrqFlags SX1278Common::waitForIrq(LockMode &guard, IrqFlags set_irq,
@@ -77,17 +76,23 @@ ISX1278::IrqFlags SX1278Common::waitForIrq(LockMode &guard, IrqFlags set_irq,
     {
         // An interrupt could occur and read from this variables
         {
-            miosix::FastInterruptDisableLock dLock;
+            miosix::FastInterruptDisableLock lock;
             state.irq_wait_thread = miosix::Thread::IRQgetCurrentThread();
         }
 
         // Check that this hasn't already happened
-        if ((ret_irq = checkForIrqAndReset(set_irq, reset_irq)) != 0)
+        if ((ret_irq = checkForIrqAndReset(set_irq, reset_irq)) != 0) 
         {
             break;
         }
 
-        waitForIrqInner(guard, unlock);
+        if(!waitForIrqInner(guard, unlock)) 
+        {
+            while(true) {
+                printf("[sx1278] Timeout!\n");
+            }
+            // TODO: Something bad happened, do something!
+        }
 
         // TODO: Check state of the device, and reset if needed!
 
@@ -129,7 +134,7 @@ ISX1278::IrqFlags SX1278Common::waitForIrqBusy(LockMode &_guard,
     return 0;
 }
 
-void SX1278Common::waitForIrqInner(LockMode &_guard, bool unlock)
+bool SX1278Common::waitForIrqInner(LockMode &_guard, bool unlock)
 {
     // Take a reference to a _guard to MAKE SURE that the mutex is locked, but
     // otherwise don't do anything with it
@@ -141,15 +146,14 @@ void SX1278Common::waitForIrqInner(LockMode &_guard, bool unlock)
         mutex.unlock();
     }
 
+    int start = miosix::getTick();
+    miosix::TimedWaitResult result = miosix::TimedWaitResult::NoTimeout;
+
     {
-        miosix::FastInterruptDisableLock dLock;
-        while (state.irq_wait_thread)
+        miosix::FastInterruptDisableLock lock;
+        while (state.irq_wait_thread && result == miosix::TimedWaitResult::NoTimeout)
         {
-            miosix::Thread::IRQwait();
-            {
-                miosix::FastInterruptEnableLock eLock(dLock);
-                miosix::Thread::yield();
-            }
+            result = miosix::Thread::IRQenableIrqAndTimedWaitMs(lock, start + IRQ_TIMEOUT);
         }
     }
 
@@ -158,6 +162,9 @@ void SX1278Common::waitForIrqInner(LockMode &_guard, bool unlock)
     {
         mutex.lock();
     }
+
+    // Check that we didn't have a timeout
+    return result == miosix::TimedWaitResult::NoTimeout;
 }
 
 ISX1278::IrqFlags SX1278Common::checkForIrqAndReset(IrqFlags set_irq,
diff --git a/src/shared/radio/SX1278/SX1278Common.h b/src/shared/radio/SX1278/SX1278Common.h
index fb142ae91..30122331f 100644
--- a/src/shared/radio/SX1278/SX1278Common.h
+++ b/src/shared/radio/SX1278/SX1278Common.h
@@ -119,6 +119,9 @@ private:
         InterruptTrigger dio1_trigger = InterruptTrigger::RISING_EDGE;
     };
 
+    // This is reasonably the maximum we should wait for an interrupt
+    static constexpr int IRQ_TIMEOUT = 100;
+
 public:
     /**
      * @brief Handle generic DIO irq.
@@ -221,7 +224,7 @@ private:
     void enableIrqs();
     void disableIrqs();
 
-    void waitForIrqInner(LockMode &guard, bool unlock);
+    bool waitForIrqInner(LockMode &guard, bool unlock);
 
     DeviceState lockMode(Mode mode, DioMapping mapping,
                          InterruptTrigger dio1_trigger, bool set_tx_frontend_on,
diff --git a/src/tests/radio/sx1278/sx1278-init.h b/src/tests/radio/sx1278/sx1278-init.h
index 3d267c8a3..36fc26a28 100644
--- a/src/tests/radio/sx1278/sx1278-init.h
+++ b/src/tests/radio/sx1278/sx1278-init.h
@@ -41,7 +41,7 @@
 // Uncomment the following line to enable Ebyte module
 // #define SX1278_IS_EBYTE
 // Uncomment the following line to ebable Skyward433 module
-#define SX1278_IS_SKYWARD433
+// #define SX1278_IS_SKYWARD433
 
 using cs   = miosix::peripherals::ra01::pc13::cs;
 using dio0 = miosix::peripherals::ra01::pc13::dio0;
@@ -91,7 +91,7 @@ using rxen                         = miosix::radio::rxEn;
 #define SX1278_IS_SKYWARD433
 
 // Comment to use SX1278_2
-#define SX1278_1
+// #define SX1278_1
 
 #ifdef SX1278_1
 using cs   = miosix::Gpio<GPIOA_BASE, 4>;
@@ -127,7 +127,7 @@ using rst = miosix::Gpio<GPIOA_BASE, 3>;
 #define SX1278_SPI SPI3
 
 #define SX1278_IRQ_DIO0 EXTI8_IRQHandlerImpl
-#define SX1278_IRQ_DIO1 stm32f429zi_skyward_groundstation_v2EXTI10_IRQHandlerImpl
+#define SX1278_IRQ_DIO1 EXTI10_IRQHandlerImpl
 #define SX1278_IRQ_DIO3 EXTI12_IRQHandlerImpl
 #endif
 
@@ -169,14 +169,14 @@ void __attribute__((used)) SX1278_IRQ_DIO3()
 
 void initBoard()
 {   
-    sck::mode(miosix::Mode::ALTERNATE);
-    sck::alternateFunction(5);
-    miso::mode(miosix::Mode::ALTERNATE);
-    miso::alternateFunction(5);
-    mosi::mode(miosix::Mode::ALTERNATE);
-    mosi::alternateFunction(5);
-    cs::mode(miosix::Mode::OUTPUT);
-    cs::high();
+    // sck::mode(miosix::Mode::ALTERNATE);
+    // sck::alternateFunction(6);
+    // miso::mode(miosix::Mode::ALTERNATE);
+    // miso::alternateFunction(6);
+    // mosi::mode(miosix::Mode::ALTERNATE);
+    // mosi::alternateFunction(5);
+    // cs::mode(miosix::Mode::OUTPUT);
+    // cs::high();
 
 #ifdef SX1278_IS_EBYTE
     rxen::mode(miosix::Mode::OUTPUT);
@@ -236,7 +236,7 @@ bool initRadio()
 
     sx1278 = new Boardcore::SX1278Fsk(sx1278_bus, cs::getPin(), dio0::getPin(),
                                             dio1::getPin(), dio3::getPin(),
-                                            Boardcore::SPI::ClockDivider::DIV_64,
+                                            Boardcore::SPI::ClockDivider::DIV_32,
                                             std::move(frontend));
 
     printf("\n[sx1278] Configuring sx1278 fsk...\n");
-- 
GitLab