misc threading/safety improvements (#2854)

* allow termination of thread controller

* no gpio if fatal

* stub for test

firmware/controllers/system/controller_base.h

@@ -1,23 +0,0 @@
-/**
- *  @file controller_base.h
- * 
- *  @date Jan 5, 2019
- *  @author Matthew Kennedy, (c) 2019
- */
-
-#pragma once
-
-// Sparse for now...
-class ControllerBase
-{
-private:
-    const char* m_name;
-
-public:
-    ControllerBase(const char* name)
-        : m_name(name)
-    {
-    }
-
-    const char* GetName() const { return m_name; }
-};

firmware/controllers/system/efi_gpio.cpp

@@ -486,6 +486,12 @@ void OutputPin::initPin(const char *msg, brain_pin_e brainPin, const pin_output_
 	// Enter a critical section so that other threads can't change the pin state out from underneath us
 	chibios_rt::CriticalSectionLocker csl;
 
+	if (hasFirmwareError()) {
+		// Don't allow initializing more pins if we have a fatal error.
+		// Pins should have just been reset, so we shouldn't try to init more.
+		return;
+	}
+
 	// Check that this OutputPin isn't already assigned to another pin (reinit is allowed to change mode)
 	// To avoid this error, call deInit() first
 	if (isBrainPinValid(this->brainPin) && this->brainPin != brainPin) {
@@ -608,7 +614,6 @@ void initPrimaryPins() {
 
 /**
  * This method is part of fatal error handling.
- * Please note that worst case scenario the pins might get re-enabled by some other code :(
  * The whole method is pretty naive, but that's at least something.
  */
 void turnAllPinsOff(void) {

firmware/controllers/system/periodic_thread_controller.h

@@ -60,8 +60,7 @@ private:
     {
         OnStarted();
 
-        while(true)
+        while(!chThdShouldTerminateX()) {
-        {
             systime_t before = chVTGetSystemTime();
             efitick_t nowNt = getTimeNowNt();
 
@@ -79,6 +78,8 @@ private:
             // doing work, so the loop runs at a predictable 500hz.
             chThdSleepUntilWindowed(before, before + m_period);
         }
+
+		firmwareError(OBD_PCM_Processor_Fault, "Thread died: %s", this->m_name);
 	}
 
 public:

firmware/controllers/system/thread_controller.h

@@ -8,7 +8,6 @@
 #pragma once
 
 #include "global.h"
-#include "controller_base.h"
 
 /**
  * @brief A base class for a controller that requires its own thread.
@@ -18,7 +17,7 @@
  * allocate the stack at compile time, it has to be a template parameter instead of a normal parameter)
  */
 template <int TStackSize>
-class ThreadController : public ControllerBase
+class ThreadController
 {
 private:
     THD_WORKING_AREA(m_threadstack, TStackSize);
@@ -44,10 +43,12 @@ protected:
     virtual void ThreadTask() = 0;
     thread_t* m_thread;
 
+	const char* const m_name;
+
 public:
     ThreadController(const char* name, tprio_t priority)
-        : ControllerBase(name)
+		: m_prio(priority)
-        , m_prio(priority)
+		, m_name(name)
     {
     }
 
@@ -57,12 +58,12 @@ public:
     void Start()
     {
 		if (m_isStarted) {
-			warning(CUSTOM_OBD_6003, "Tried to start thread %s but it was already running", GetName());
+			warning(CUSTOM_OBD_6003, "Tried to start thread %s but it was already running", m_name);
 			return;
 		}
 
         m_thread = chThdCreateStatic(m_threadstack, sizeof(m_threadstack), m_prio, StaticThreadTaskAdapter, this);
-        m_thread->name = GetName();
+		m_thread->name = m_name;
 		m_isStarted = true;
     }
 };

unit_tests/chibios-mock/mock-threads.h

@@ -14,6 +14,7 @@ public:
 
 };
 
+bool chThdShouldTerminateX(void);
 systime_t chVTGetSystemTime(void);
 systime_t chThdSleepUntilWindowed(systime_t prev, systime_t next);
 

unit_tests/tests/system/test_periodic_thread_controller.cpp

@@ -28,6 +28,10 @@ systime_t chThdSleepUntilWindowed(systime_t prev, systime_t next) {
 	return 0;
 }
 
+bool chThdShouldTerminateX(void) {
+	return false;
+}
+
 class taskq {
 public:
 //  int trigger(taskq &tq);