Some minor fixes based on futher review of recent commits

Author: dkulp

src/OutputMonitor.cpp

@@ -596,7 +596,9 @@ void OutputMonitor::RemovePortConfiguration(int port, const Json::Value& config)
             // fusePins is keyed by the interrupt pin's name -- erasing it here
             // is what lets a later AddPortConfiguration re-register the
             // callback; leaving it behind means fuse trips are silently
-            // ignored after a config reload.
+            // ignored after a config reload.  getPinByName() matches on name
+            // exactly (there are no aliases), so the name here is the same
+            // string the add side keyed on.
             bool sharedInterrupt = false;
             for (auto other : portPins) {
                 if (other && other != pi && other->eFuseInterruptPin == pi->eFuseInterruptPin) {
@@ -665,9 +667,14 @@ void OutputMonitor::AddPortConfiguration(int port, const Json::Value& pinConfig,
     if (pinConfig.isMember("eFusePin")) {
         if (pinConfig.isMember("eFuseInterruptPin")) {
             std::string eFuseInterruptPin = pinConfig.get("eFuseInterruptPin", "").asString();
-            bool eFuseInterruptHigh = false;
+            // A leading '!' is stripped but carries no meaning here, and there
+            // is deliberately no inverted-interrupt flag to go with it: the
+            // callback below ignores the edge value it is handed and re-reads
+            // every affected port's eFusePin, so which edge announced the trip
+            // cannot change the outcome.  (The pin is registered for BOTH edges
+            // anyway -- some i2c expanders mislabel a single-edge request -- so
+            // a polarity flag would have nothing to act on.)
             if (eFuseInterruptPin[0] == '!') {
-                eFuseInterruptHigh = true;
                 eFuseInterruptPin = eFuseInterruptPin.substr(1);
             }
             std::string postFix = "";

src/Scheduler.cpp

@@ -75,6 +75,16 @@ Scheduler::Scheduler() :
     }
 
     m_lastProcTime = time(NULL);
+    // Seed the FPP-command catch-up window to now rather than leaving it at the
+    // epoch.  CheckScheduledItems() fires any command whose second falls in
+    // (m_lastCommandCheckTime, now], which exists so a second this process
+    // MISSED -- a slow midnight reload, an NTP step -- still runs.  A second
+    // that passed before this process existed was never missed by it, and the
+    // 60s catch-up cap is not a substitute: from 0 every start would replay the
+    // preceding minute of commands, so a scheduled "Outputs Off" or a restart
+    // would run a second time on every restart, and repeatedly in a restart
+    // loop.
+    m_lastCommandCheckTime = m_lastProcTime;
     LoadScheduleFromFile();
 }
 
@@ -1472,11 +1482,20 @@ static bool ParseIntArg(const std::string& str, int& out) {
     errno = 0;
     long v = strtol(start, &end, 10);
 
+    // "Consumed nothing" has to be decided BEFORE the trailing-whitespace skip
+    // below moves `end`: strtol() skips leading whitespace itself, so an
+    // all-whitespace argument leaves end == start with v == 0, and skipping
+    // first walks end to the terminator and makes the test pass -- reporting a
+    // successful parse of 0 for an argument that held no number at all.
+    if (end == start) {
+        return false;
+    }
+
     while ((*end == ' ') || (*end == '\t')) {
         end++;
     }
 
-    if ((end == start) || (*end != '\0') || (errno == ERANGE) ||
+    if ((*end != '\0') || (errno == ERANGE) ||
         (v < INT_MIN) || (v > INT_MAX)) {
         return false;
     }

src/Sequence.cpp

@@ -504,17 +504,21 @@ void Sequence::queuePreset(const std::string& preset, std::map<std::string, std:
     // with no sequence locks held; a preset that re-enters the sequence (or
     // queues further presets) is therefore safe.
     Timers::INSTANCE.addTimer("SequencePresets", GetTimeMS(), [this]() {
-        std::unique_lock<std::mutex> l(m_pendingPresetsLock);
-        while (!m_pendingPresets.empty()) {
-            auto p = std::move(m_pendingPresets.front());
-            m_pendingPresets.pop_front();
-            l.unlock();
-            CommandManager::INSTANCE.TriggerPreset(p.first, p.second);
-            l.lock();
-        }
+        drainPendingPresets();
     });
 }
 
+void Sequence::drainPendingPresets() {
+    std::unique_lock<std::mutex> l(m_pendingPresetsLock);
+    while (!m_pendingPresets.empty()) {
+        auto p = std::move(m_pendingPresets.front());
+        m_pendingPresets.pop_front();
+        l.unlock();
+        CommandManager::INSTANCE.TriggerPreset(p.first, p.second);
+        l.lock();
+    }
+}
+
 void Sequence::StartSequence() {
     // Callers reach this without m_sequenceLock (the command handler and the
     // playlist), so the name has to be snapshotted rather than read raw.

src/Sequence.h

@@ -73,6 +73,14 @@ class Sequence {
     // freed buffer; copying under m_seqFilenameLock is the only safe read.
     std::string GetSeqFilenameCopy() const;
 
+    // Run every queued sequence preset now, on the calling thread.  Normally
+    // driven by the main-loop timer queuePreset() arms; called directly once
+    // during shutdown, because a preset queued in the last loop iteration
+    // (a sequence that ended, or a stop command, right as the loop exits) has
+    // no later tick to fire on and would otherwise be dropped.  MUST be called
+    // with no sequence lock held -- presets run arbitrary commands.
+    void drainPendingPresets();
+
     int GetSeqStepTime() const { return m_seqStepTime; }
     void BlankSequenceData(bool clearBridge = false);
 

src/fppd.cpp

@@ -1122,6 +1122,17 @@ int main(int argc, char* argv[]) {
     // incomplete and cause problems with summary
     // PublishStatsForce("Shutdown"); // not background
 
+    // Sequence presets (SEQUENCE_STOPPED and the FSEQ frame-triggered commands)
+    // are queued for the main loop rather than run at their call site, which
+    // holds the sequence locks.  A preset queued during the final loop
+    // iteration -- a sequence that ended, or a stop command, landing just as
+    // runMainFPPDLoop goes to 0 -- has no later tick to fire on.  Drain it here
+    // instead: still before FPPD_STOPPED, so the ordering a user would expect
+    // between the two is preserved, and well before CommandManager::Cleanup().
+    if (sequence) {
+        sequence->drainPendingPresets();
+    }
+
     if (CommandManager::INSTANCE.HasPreset("FPPD_STOPPED")) {
         CommandManager::INSTANCE.TriggerPreset("FPPD_STOPPED");
     }