Optimizes the implementation of the railcom driver for multi-channel adapters (#865)

- reduces the number of lock-unlock sequences needed for a packet allocation
Adjusts for new overrides:
- moves some symbols to public which different implementations will need
- Adds a hook for the cutout behavior on a per-hardware basis.

In Stm32 implementation:
- Makes the time window for sampling resistor smaller during occupancy sample measurement.
- Adds support for allocating feedback packets slightly earlier because this takes a lot of time.
- removes redundant code when no cutout is present.

===

* Optimizes the implementation of the railcom driver for multi-channel adapters:
- reduces the number of lock-unlock sequences needed for a packet allocation
Adjusts for new overrides:
- moves some symbols to public which different implementations will need
- Adds a hook for the cutout behavior on a per-hardware basis.

In Stm32 implementation:
- Makes the time window for sampling resistor smaller during occupancy sample measurement.
- Adds support for allocating feedback packets slightly earlier because this takes a lot of time.
- removes redundant code when no cutout is present.

* Fix whitespace.

* Add an enum for the DccDecoder::CutoutState.
Fix comments.

* Update more references to cutout state.

* Fix typo in comment.

Author: balazsracz

src/freertos_drivers/common/DccDecoder.hxx

@@ -38,6 +38,28 @@
 #include "dcc/PacketProcessor.hxx"
 #include "dcc/packet.h"
 
+struct DccDecoderDefs
+{
+    /// Allowed values of DccDecoder::cutoutState_.
+    enum CutoutState : uint8_t
+    {
+        /// Next wakeup of the usec timer will be the beginning of the cutout
+        /// (nominally 26 usec after the last 1 bit), when the cutout FET
+        /// should be turned on.
+        CUTOUT_BEGIN,
+        /// Next wakeup of the cutout timer is the middle between the two
+        /// windows.
+        CUTOUT_MID,
+        /// Next wakeup of the cutout timer is the end of the cutout, when the
+        /// railcom fet should be switched off and the booster output be
+        /// reenabled.
+        CUTOUT_END,
+        /// Alias for the cutout end, but do not call the railcomDriver and do
+        /// not reset the module.
+        CUTOUT_END_AFTER_DRIVER
+    };
+}; // struct DccDecoderDefs
+
 /**
   Device driver for decoding a DCC signal using a Timer resource.
 
@@ -65,6 +87,8 @@
   - SAMPLE_PERIOD_CLOCKS
   - Q_SIZE
   - TICKS_PER_USEC
+  - Output, which is a DccOutput compatible static object for creating
+      Railcom cutout.
   - module_init()
   - module_enable()
   - module_disable()
@@ -84,6 +108,8 @@
 template <class Module> class DccDecoder : public Node
 {
 public:
+    using Defs = DccDecoderDefs;
+    
     /// Constructor.
     ///
     /// @param name name of device node, e.g. "/dev/dccdecode0"
@@ -113,6 +139,16 @@ public:
     /** Handles a software interrupt to FreeRTOS. */
     inline void os_interrupt_handler() __attribute__((always_inline));
 
+    /// How many usec the railcom has before the cutout (measured from the
+    /// packet end 1 bit complete)
+    static const auto RAILCOM_CUTOUT_PRE = 26;
+    /// How many usec the railcom has to the middle of window (measured from the
+    /// packet end 1 bit complete)
+    static const auto RAILCOM_CUTOUT_MID = 185;
+    /// How many usec the railcom has to the end of the window (measured from
+    /// the packet end 1 bit complete)
+    static const auto RAILCOM_CUTOUT_END = 471;
+
 private:
     /** Read from a file or device.
      * @param file file reference for this device
@@ -224,7 +260,7 @@ private:
     /// colliding with cutout.
     bool prepCutout_ = false;
     /// Which window of the cutout we are in.
-    uint32_t cutoutState_;
+    Defs::CutoutState cutoutState_;
     /// Counts unique identifiers for DCC packets to be returned.
     uint32_t packetId_ = 0;
     /// How many times did we lose a DCC packet due to no buffer available.
@@ -239,16 +275,6 @@ private:
     /// DCC packet decoder state machine and internal state.
     dcc::DccDecoder decoder_ {Module::get_ticks_per_usec()};
 
-    /// How many usec the railcom has before the cutout (measured from the
-    /// packet end 1 bit complete)
-    static const auto RAILCOM_CUTOUT_PRE = 26;
-    /// How many usec the railcom has to the middle of window (measured from the
-    /// packet end 1 bit complete)
-    static const auto RAILCOM_CUTOUT_MID = 185;
-    /// How many usec the railcom has to the end of the window (measured from
-    /// the packet end 1 bit complete)
-    static const auto RAILCOM_CUTOUT_END = 471;
-
     DISALLOW_COPY_AND_ASSIGN(DccDecoder);
 };
 
@@ -324,9 +350,24 @@ __attribute__((optimize("-O3"))) void DccDecoder<Module>::interrupt_handler()
         debugLog_.add(new_value);
 #endif        
         bool cutout_just_finished = false;
+        Debug::DccDecodeInterrupts::set(false);
         decoder_.process_data(new_value);
+        Debug::DccDecodeInterrupts::set(true);
+        if (decoder_.state() == dcc::DccDecoder::DCC_END_OF_PREAMBLE)
+        {
+            // Resets these state bits in case the state machines have
+            // diverged due to a bug.
+            inCutout_ = false;
+            prepCutout_ = false;
+            cutoutState_ = DccDecoderDefs::CUTOUT_BEGIN;
+        }
+        if (decoder_.state() == dcc::DccDecoder::DCC_MAYBE_CUTOUT)
+        {
+            Debug::DccInCutoutPin::set(true);
+        }
         if (decoder_.before_dcc_cutout())
         {
+            Debug::RailComBeforeCutoutTiming::set(true);
             prepCutout_ = true;
             auto* p = decoder_.pkt();
             if (p)
@@ -345,11 +386,12 @@ __attribute__((optimize("-O3"))) void DccDecoder<Module>::interrupt_handler()
             true) // Module::NRZ_Pin::get())
         {
             //Debug::RailcomDriverCutout::set(true);
+            Debug::RailComBeforeCutoutTiming::set(false);
             Module::set_cap_timer_time();
             Module::set_cap_timer_delay_usec(
                 RAILCOM_CUTOUT_PRE + Module::time_delta_railcom_pre_usec());
             inCutout_ = true;
-            cutoutState_ = 0;
+            cutoutState_ = DccDecoderDefs::CUTOUT_BEGIN;
             if (decoder_.pkt())
             {
                 nextPacketFilled_ = true;
@@ -361,21 +403,23 @@ __attribute__((optimize("-O3"))) void DccDecoder<Module>::interrupt_handler()
             //railcomDriver_->start_cutout();
             //inCutout_ = true;
         }
-        else if (decoder_.state() == dcc::DccDecoder::DCC_PACKET_FINISHED)
+        else if ((decoder_.state() == dcc::DccDecoder::DCC_PACKET_FINISHED &&
+                     !inCutout_) ||
+            (decoder_.state() == dcc::DccDecoder::DCC_PREAMBLE && prepCutout_ &&
+                !inCutout_))
         {
+            // The first partial bit after the cutout is done (for upstream
+            // cutout), or the preamble bit which came after the
+            // locally-generated cutout was finished.
             Debug::DccPacketFinishedHook::set(true);
-            if (inCutout_) {
-                //railcomDriver_->end_cutout();
-                inCutout_ = false;
-            }
             Module::dcc_packet_finished_hook();
             prepCutout_ = false;
             cutout_just_finished = true;
             Debug::DccPacketFinishedHook::set(false);
         }
         lastTimerValue_ = raw_new_value;
-        if (sampleActive_ && Module::NRZ_Pin::get() && !prepCutout_ &&
-            !cutout_just_finished)
+        if (sampleActive_ // && !inCutout_
+            && !prepCutout_ && !cutout_just_finished && Module::NRZ_Pin::get())
         {
             sampleActive_ = false;
             // The first positive edge after the sample timer expired (but
@@ -384,6 +428,8 @@ __attribute__((optimize("-O3"))) void DccDecoder<Module>::interrupt_handler()
             Module::after_feedback_hook();
         }
     }
+    Debug::DccInCutoutPin::set(inCutout_);
+    Debug::DccDecodeInterrupts::set(false);
 }
 
 template <class Module>
@@ -394,35 +440,67 @@ DccDecoder<Module>::rcom_interrupt_handler()
     if (Module::int_get_and_clear_delay_event())
     {
         // Debug::RailcomDriverCutout::set(false);
+        Module::rcom_cutout_hook(&cutoutState_);
         switch (cutoutState_)
         {
-            case 0:
+            case Defs::CUTOUT_BEGIN:
             {
                 Module::set_cap_timer_delay_usec(
                     RAILCOM_CUTOUT_MID + Module::time_delta_railcom_mid_usec());
+                if (Module::Output::need_railcom_cutout())
+                {
+                    Debug::RailcomTurnonPhase1::set(true);
+                    unsigned delay_usec =
+                        Module::Output::start_railcom_cutout_phase1();
+                    Module::Output::isRailcomCutoutActive_ = 1;
+                    microdelay(delay_usec);
+                    Debug::RailcomTurnonPhase1::set(false);
+                    delay_usec = Module::Output::start_railcom_cutout_phase2();
+                    microdelay(delay_usec);
+                }
                 railcomDriver_->start_cutout();
-                cutoutState_ = 1;
+                cutoutState_ = Defs::CUTOUT_MID;
                 break;
             }
-            case 1:
+            case Defs::CUTOUT_MID:
             {
                 Module::set_cap_timer_delay_usec(
                     RAILCOM_CUTOUT_END + Module::time_delta_railcom_end_usec());
                 railcomDriver_->middle_cutout();
-                cutoutState_ = 2;
+                cutoutState_ = Defs::CUTOUT_END;
                 break;
             }
-            default:
+            case Defs::CUTOUT_END:
             {
                 Module::stop_cap_timer_time();
                 Module::set_cap_timer_capture();
                 railcomDriver_->end_cutout();
+            } // fall through
+            case Defs::CUTOUT_END_AFTER_DRIVER:
+            {
+                if (Module::Output::isRailcomCutoutActive_)
+                {
+                    Debug::RailcomTurnonPhase1::set(true);
+                    unsigned delay_usec =
+                        Module::Output::stop_railcom_cutout_phase1();
+                    microdelay(delay_usec);
+                    Debug::RailcomTurnonPhase1::set(false);
+                    Module::Output::stop_railcom_cutout_phase2();
+                    Module::Output::isRailcomCutoutActive_ = 0;
+                }
                 inCutout_ = false;
+                if (Module::Output::should_be_enabled())
+                {
+                    Module::Output::enable_output();
+                }
                 break;
             }
+            default:
+                break;
         }
     }
     Debug::DccDecodeInterrupts::set(false);
+    Debug::DccInCutoutPin::set(inCutout_);
 }
 
 template <class Module>

src/freertos_drivers/common/FixedQueue.hxx

@@ -134,6 +134,27 @@ public:
         __atomic_fetch_add(&count_, 1, __ATOMIC_SEQ_CST);
     }
 
+    /// Checks if there is space in the back. If yes, allocates one entry as
+    /// noncommit space and returns the pointer. If full, returns nullptr.
+    /// @return nullptr if the queue is full, otherwise a noncommit entry.
+    T *noncommit_back_or_null()
+    {
+        auto sz = size();
+        if (sz >= SIZE)
+        {
+            return nullptr;
+        }
+        if (sz != 0 && rdIndex_ == wrIndex_)
+        {
+            // noncommit members make the queue full.
+            return nullptr;
+        }
+        auto *ret = &storage_[wrIndex_];
+        if (++wrIndex_ >= SIZE)
+            wrIndex_ = 0;
+        return ret;
+    }
+
 private:
     /// Payload of elements stored.
     T storage_[SIZE];

src/freertos_drivers/common/RailcomImpl.hxx

@@ -222,26 +222,24 @@ public:
         }
     }
 
-private:
+protected:
     void set_feedback_key(uint32_t key) OVERRIDE
     {
         feedbackKey_ = key;
     }
 
-protected:
     /** Takes a new empty packet at the front of the queue, fills in feedback
      * key and channel information.
      * @param channel is which channel to set the packet for.
      * @return a packet pointer to write into. If there is no space, returns
      * nullptr.*/
     dcc::Feedback *alloc_new_packet(uint8_t channel)
     {
-        if (!feedbackQueue_.has_noncommit_space())
+        dcc::Feedback *entry = feedbackQueue_.noncommit_back_or_null();
+        if (!entry)
         {
             return nullptr;
         }
-        dcc::Feedback *entry = &feedbackQueue_.back();
-        feedbackQueue_.noncommit_back();
         entry->reset(feedbackKey_);
         entry->channel = channel;
         return entry;

src/freertos_drivers/st/Stm32DCCDecoder.hxx

@@ -115,6 +115,9 @@ struct DccDecoderHW
 template <class HW> class Stm32DccTimerModule
 {
 public:
+    // Declaration matching the DccOutput static class pattern.
+    using Output = typename HW::Output;
+
     /// Exports the input pin to the driver on the module interface.
     using NRZ_Pin = typename HW::NRZ_Pin;
 
@@ -261,6 +264,13 @@ public:
         //TIM_CCxChannelCmd(usec_timer(), HW::USEC_CHANNEL, TIM_CCx_DISABLE);
     }
 
+    /// Called during the railcom cutout when the railcom usec timer expires.
+    /// The implementation is expected to modify cutout_state if the default
+    /// sequence of handlers are not matching the desired
+    /// functionality. Additional wakeups can be scheduled with
+    /// set_cap_timer_delay_usec(...). An empty implementation is acceptable.
+    static inline void rcom_cutout_hook(DccDecoderDefs::CutoutState *cutout_state);
+
 private:
     static TIM_HandleTypeDef captureTimerHandle_;
     static TIM_HandleTypeDef usecTimerHandle_;
@@ -409,8 +419,8 @@ template <class HW> void Stm32DccTimerModule<HW>::module_enable()
     __HAL_TIM_DISABLE_IT(usec_timer_handle(), HW::USEC_IF);
 
 #if defined(GCC_ARMCM0)
-    HAL_NVIC_SetPriority(HW::CAPTURE_IRQn, 0, 0);
-    HAL_NVIC_SetPriority(HW::TIMER_IRQn, 0, 0);
+    HAL_NVIC_SetPriority(HW::CAPTURE_IRQn, 1, 0);
+    HAL_NVIC_SetPriority(HW::TIMER_IRQn, 1, 0);
     HAL_NVIC_SetPriority(HW::OS_IRQn, 3, 0);
 #elif defined(GCC_ARMCM3)
     SetInterruptPriority(HW::CAPTURE_IRQn, 0x20);