Optimize filament sensor implementation

- remove virtual methods (we only have one fsensor implementation at a time)
- comment out some of the debugging texts
- remove volatile and replace them with proper synchronized access to relevant variables

Firmware/Filament_sensor.cpp

@@ -41,9 +41,9 @@ FSensorBlockRunout::~FSensorBlockRunout() { }
 void Filament_sensor::setEnabled(bool enabled) {
     eeprom_update_byte((uint8_t *)EEPROM_FSENSOR, enabled);
     if (enabled) {
-        init();
+        fsensor.init();
     } else {
-        deinit();
+        fsensor.deinit();
     }
 }
 
@@ -89,16 +89,16 @@ bool Filament_sensor::checkFilamentEvents() {
         return false;
     }
 
-    bool newFilamentPresent = getFilamentPresent();
+    bool newFilamentPresent = fsensor.getFilamentPresent();
     if (oldFilamentPresent != newFilamentPresent) {
         oldFilamentPresent = newFilamentPresent;
         eventBlankingTimer.start();
         if (newFilamentPresent) { // filament insertion
-            puts_P(PSTR("filament inserted"));
+//            puts_P(PSTR("filament inserted"));
             triggerFilamentInserted();
             postponedLoadEvent = true;
         } else { // filament removal
-            puts_P(PSTR("filament removed"));
+//            puts_P(PSTR("filament removed"));
             triggerFilamentRemoved();
         }
         return true;
@@ -111,7 +111,7 @@ void Filament_sensor::triggerFilamentInserted() {
         && (eFilamentAction == FilamentAction::None)
         && (! MMU2::mmu2.Enabled() ) // quick and dirty hack to prevent spurious runouts while the MMU is in charge
         && !(
-            moves_planned()
+            moves_planned() != 0
             || IS_SD_PRINTING
             || usb_timer.running()
             || (lcd_commands_type == LcdCommands::Layer1Cal)
@@ -129,7 +129,7 @@ void Filament_sensor::triggerFilamentRemoved() {
         && (eFilamentAction == FilamentAction::None)
         && !saved_printing
         && (
-            moves_planned()
+            moves_planned() != 0
             || IS_SD_PRINTING
             || usb_timer.running()
             || (lcd_commands_type == LcdCommands::Layer1Cal)
@@ -175,16 +175,16 @@ void Filament_sensor::triggerError() {
 #if (FILAMENT_SENSOR_TYPE == FSENSOR_IR) || (FILAMENT_SENSOR_TYPE == FSENSOR_IR_ANALOG)
 void IR_sensor::init() {
     if (state == State::error) {
-        deinit(); // deinit first if there was an error.
+        fsensor.deinit(); // deinit first if there was an error.
     }
-    puts_P(PSTR("fsensor::init()"));
+//    puts_P(PSTR("fsensor::init()"));
     SET_INPUT(IR_SENSOR_PIN); // input mode
     WRITE(IR_SENSOR_PIN, 1);  // pullup
     settings_init();          // also sets the state to State::initializing
 }
 
 void IR_sensor::deinit() {
-    puts_P(PSTR("fsensor::deinit()"));
+//    puts_P(PSTR("fsensor::deinit()"));
     SET_INPUT(IR_SENSOR_PIN); // input mode
     WRITE(IR_SENSOR_PIN, 0);  // no pullup
     state = State::disabled;
@@ -194,16 +194,12 @@ bool IR_sensor::update() {
     switch (state) {
     case State::initializing:
         state = State::ready; // the IR sensor gets ready instantly as it's just a gpio read operation.
-        oldFilamentPresent =
-            getFilamentPresent(); // initialize the current filament state so that we don't create a switching event right after the sensor is ready.
-        // fallthru
+        // initialize the current filament state so that we don't create a switching event right after the sensor is ready.
+        oldFilamentPresent = fsensor.getFilamentPresent();
+        [[fallthrough]];
     case State::ready: {
         postponedLoadEvent = false;
-        bool event = checkFilamentEvents();
-
-        ; //
-
-        return event;
+        return checkFilamentEvents();
     } break;
     case State::disabled:
     case State::error:
@@ -213,7 +209,7 @@ bool IR_sensor::update() {
     return false;
 }
 
-bool IR_sensor::getFilamentPresent() { return !READ(IR_SENSOR_PIN); }
+
 
 #ifdef FSENSOR_PROBING
 bool IR_sensor::probeOtherType() { return pat9125_probe(); }
@@ -224,16 +220,15 @@ void IR_sensor::settings_init() { Filament_sensor::settings_init_common(); }
 #if (FILAMENT_SENSOR_TYPE == FSENSOR_IR_ANALOG)
 void IR_sensor_analog::init() {
     IR_sensor::init();
-    settings_init();
+    IR_sensor::settings_init();
+    sensorRevision = (SensorRevision)eeprom_read_byte((uint8_t *)EEPROM_FSENSOR_PCB);
 }
 
-void IR_sensor_analog::deinit() { IR_sensor::deinit(); }
-
 bool IR_sensor_analog::update() {
     bool event = IR_sensor::update();
     if (state == State::ready) {
-        if (voltReady) {
-            voltReady = false;
+        if (getVoltReady()) {
+            clearVoltReady();
             uint16_t volt = getVoltRaw();
 //            printf_P(PSTR("newVoltRaw:%u\n"), volt / OVERSAMPLENR);
 
@@ -282,14 +277,7 @@ void IR_sensor_analog::voltUpdate(uint16_t raw) { // to be called from the ADC I
 }
 
 uint16_t IR_sensor_analog::getVoltRaw() {
-    uint16_t newVoltRaw;
-    ATOMIC_BLOCK(ATOMIC_RESTORESTATE) { newVoltRaw = voltRaw; }
-    return newVoltRaw;
-}
-
-void IR_sensor_analog::settings_init() {
-    IR_sensor::settings_init();
-    sensorRevision = (SensorRevision)eeprom_read_byte((uint8_t *)EEPROM_FSENSOR_PCB);
+    ATOMIC_BLOCK(ATOMIC_RESTORESTATE) { return voltRaw; }
 }
 
 const char *IR_sensor_analog::getIRVersionText() {
@@ -349,6 +337,14 @@ bool IR_sensor_analog::checkVoltage(uint16_t raw) {
     return true;
 }
 
+bool IR_sensor_analog::getVoltReady() const {
+    ATOMIC_BLOCK(ATOMIC_RESTORESTATE){ return voltReady; }
+}
+
+void IR_sensor_analog::clearVoltReady(){
+    ATOMIC_BLOCK(ATOMIC_RESTORESTATE){ voltReady = false; }
+}
+
 void IR_sensor_analog::IR_ANALOG_Check(SensorRevision isVersion, SensorRevision switchTo) {
     bool bTemp = (!CHECK_ALL_HEATERS);
     bTemp = bTemp && (menu_menu == lcd_status_screen);
@@ -383,7 +379,7 @@ void PAT9125_sensor::init() {
     if (state == State::error) {
         deinit(); // deinit first if there was an error.
     }
-    puts_P(PSTR("fsensor::init()"));
+//    puts_P(PSTR("fsensor::init()"));
 
     settings_init(); // also sets the state to State::initializing
 
@@ -402,7 +398,7 @@ void PAT9125_sensor::init() {
 }
 
 void PAT9125_sensor::deinit() {
-    puts_P(PSTR("fsensor::deinit()"));
+//    puts_P(PSTR("fsensor::deinit()"));
     ; //
     state = State::disabled;
     filter = 0;
@@ -459,15 +455,13 @@ void PAT9125_sensor::setJamDetectionEnabled(bool state, bool updateEEPROM) {
 }
 
 void PAT9125_sensor::settings_init() {
-    puts_P(PSTR("settings_init"));
+//    puts_P(PSTR("settings_init"));
     Filament_sensor::settings_init_common();
     setJamDetectionEnabled(eeprom_read_byte((uint8_t *)EEPROM_FSENSOR_JAM_DETECTION));
 }
 
 int16_t PAT9125_sensor::getStepCount() {
-    int16_t st_cnt;
-    ATOMIC_BLOCK(ATOMIC_RESTORESTATE) { st_cnt = stepCount; }
-    return st_cnt;
+    ATOMIC_BLOCK(ATOMIC_RESTORESTATE) { return stepCount; }
 }
 
 void PAT9125_sensor::resetStepCount() {

Firmware/Filament_sensor.h

@@ -20,17 +20,17 @@ public:
     ~FSensorBlockRunout();
 };
 
+/// Base class Filament sensor
+/// 
+/// Ideally, there could have been a nice class hierarchy of filament sensor types with common functionality
+/// extracted into this base class.
+/// But:
+/// - virtual methods take more space
+/// - we don't need to switch among different filament sensors at runtime
+/// Therefore the class hierarchy carefully avoids using virtual methods and doesn't look too fancy.
 #ifdef FILAMENT_SENSOR
 class Filament_sensor {
 public:
-    virtual void init() = 0;
-    virtual void deinit() = 0;
-    virtual bool update() = 0;
-    virtual bool getFilamentPresent() = 0;
-#ifdef FSENSOR_PROBING
-    virtual bool probeOtherType() = 0; //checks if the wrong fsensor type is detected.
-#endif
-    
     enum class State : uint8_t {
         disabled = 0,
         initializing,
@@ -44,41 +44,22 @@ public:
         _Undef = EEPROM_EMPTY_VALUE
     };
     
-    void setEnabled(bool enabled);
+    static void setEnabled(bool enabled);
     
     void setAutoLoadEnabled(bool state, bool updateEEPROM = false);
-    
-    bool getAutoLoadEnabled() {
-        return autoLoadEnabled;
-    }
+    bool getAutoLoadEnabled() const { return autoLoadEnabled; }
     
     void setRunoutEnabled(bool state, bool updateEEPROM = false);
-    
-    bool getRunoutEnabled() {
-        return runoutEnabled;
-    }
+    bool getRunoutEnabled() const { return runoutEnabled; }
     
     void setActionOnError(SensorActionOnError state, bool updateEEPROM = false);
+    SensorActionOnError getActionOnError() const { return sensorActionOnError; }
     
-    SensorActionOnError getActionOnError() {
-        return sensorActionOnError;
-    }
-    
-    bool getFilamentLoadEvent() {
-        return postponedLoadEvent;
-    }
-    
-    bool isError() {
-        return state == State::error;
-    }
-    
-    bool isReady() {
-        return state == State::ready;
-    }
+    bool getFilamentLoadEvent() const { return postponedLoadEvent; }
     
-    bool isEnabled() {
-        return state != State::disabled;
-    }
+    bool isError() const { return state == State::error; }
+    bool isReady() const { return state == State::ready; }
+    bool isEnabled() const { return state != State::disabled; }
     
 protected:
     void settings_init_common();
@@ -89,7 +70,7 @@ protected:
     
     void triggerFilamentRemoved();
     
-    void filAutoLoad();
+    static void filAutoLoad();
     
     void filRunout();
     
@@ -107,12 +88,12 @@ protected:
 #if (FILAMENT_SENSOR_TYPE == FSENSOR_IR) || (FILAMENT_SENSOR_TYPE == FSENSOR_IR_ANALOG)
 class IR_sensor: public Filament_sensor {
 public:
-    void init() override;
-    void deinit() override;
-    bool update()override ;
-    bool getFilamentPresent()override;
+    void init();
+    void deinit();
+    bool update();
+    bool getFilamentPresent() const { return !READ(IR_SENSOR_PIN); }
 #ifdef FSENSOR_PROBING
-    bool probeOtherType()override;
+    static bool probeOtherType(); //checks if the wrong fsensor type is detected.
 #endif
     void settings_init();
 };
@@ -127,14 +108,11 @@ constexpr static float Raw2Voltage(uint16_t raw) {
 
 class IR_sensor_analog: public IR_sensor {
 public:
-    void init()override;
-    void deinit()override;
-    bool update()override;
+    void init();
+    bool update();
     void voltUpdate(uint16_t raw);
     
-    uint16_t getVoltRaw();
-    
-    void settings_init();
+    uint16_t __attribute__((noinline)) getVoltRaw();
     
     enum class SensorRevision : uint8_t {
         _Old = 0,
@@ -142,17 +120,12 @@ public:
         _Undef = EEPROM_EMPTY_VALUE
     };
     
-    SensorRevision getSensorRevision() {
-        return sensorRevision;
-    }
+    SensorRevision getSensorRevision() const { return sensorRevision; }
     
-    const char* getIRVersionText();
+    const char* __attribute__((noinline)) getIRVersionText();
     
     void setSensorRevision(SensorRevision rev, bool updateEEPROM = false);
     
-    bool checkVoltage(uint16_t raw);
-    
-    // Voltage2Raw is not constexpr :/
     constexpr static uint16_t IRsensor_Ldiode_TRESHOLD = Voltage2Raw(0.3F); // ~0.3V, raw value=982
     constexpr static uint16_t IRsensor_Lmax_TRESHOLD = Voltage2Raw(1.5F); // ~1.5V (0.3*Vcc), raw value=4910
     constexpr static uint16_t IRsensor_Hmin_TRESHOLD = Voltage2Raw(3.0F); // ~3.0V (0.6*Vcc), raw value=9821
@@ -161,8 +134,14 @@ public:
     
 private:
     SensorRevision sensorRevision;
-    volatile bool voltReady; //this gets set by the adc ISR
-    volatile uint16_t voltRaw;
+    
+    bool voltReady; // set by the adc ISR, therefore avoid accessing the variable directly but use getVoltReady()
+    bool getVoltReady()const;
+    void clearVoltReady();
+    
+    uint16_t voltRaw; // set by the adc ISR, therefore avoid accessing the variable directly but use getVoltRaw()
+    bool checkVoltage(uint16_t raw);
+    
     uint16_t minVolt = Voltage2Raw(6.F);
     uint16_t maxVolt = 0;
     uint16_t nFSCheckCount;
@@ -181,22 +160,16 @@ private:
 #if (FILAMENT_SENSOR_TYPE == FSENSOR_PAT9125)
 class PAT9125_sensor: public Filament_sensor {
 public:
-    void init()override;
-    void deinit()override;
-    bool update()override;
-    bool getFilamentPresent() override{
-        return filterFilPresent;
-    }
-    
+    void init();
+    void deinit();
+    bool update();
+    bool getFilamentPresent() const { return filterFilPresent; }
 #ifdef FSENSOR_PROBING
-    bool probeOtherType() override;
+    bool probeOtherType(); //checks if the wrong fsensor type is detected.
 #endif
     
     void setJamDetectionEnabled(bool state, bool updateEEPROM = false);
-    
-    bool getJamDetectionEnabled() {
-        return jamDetection;
-    }
+    bool getJamDetectionEnabled() const { return jamDetection; }
     
     void stStep(bool rev) { //from stepper isr
         stepCount += rev ? -1 : 1;
@@ -212,7 +185,7 @@ private:
     
     bool jamDetection;
     int16_t oldPos;
-    volatile int16_t stepCount;
+    int16_t stepCount;
     int16_t chunkSteps;
     uint8_t jamErrCnt;