Fix handling of uncertain bits in accessories

When the state of an accessory is first set, send commands for all
uncertain bits.  Otherwise the accessory could remain unsynced or a
failure could go undetected.

Always clear the uncertain bit after setting an accessory, not only
when it failed.

diff --git a/source/libr2c2/arducontrol.cpp b/source/libr2c2/arducontrol.cpp
index 98f74338285178ecb73a11e63faa005deea56f6e..2cdf0c6798440026782b7ba3b9d2ca245b74db9a 100644 (file)
--- a/source/libr2c2/arducontrol.cpp
+++ b/source/libr2c2/arducontrol.cpp
@@ -276,7 +276,7 @@ void ArduControl::set_accessory(Accessory::Kind kind, unsigned addr, unsigned st
        if(acc.kind!=kind)
                throw key_error(addr);
 
-       if(state!=acc.target)
+       if(state!=acc.target || acc.uncertain)
        {
                acc.target = state;
                accessory_queue.push_back(&acc);
@@ -378,13 +378,6 @@ void ArduControl::tick()
                        {
                                if(acc.state.commit(tag.serial))
                                {
-                                       if(acc.state==acc.target)
-                                       {
-                                               if(acc.kind==Accessory::TURNOUT)
-                                                       signal_turnout.emit(acc.address, acc.state);
-                                               else if(acc.kind==Accessory::SIGNAL)
-                                                       signal_signal.emit(acc.address, acc.state);
-                                       }
                                        if(&acc==active_accessory)
                                                active_accessory = 0;
                                }
@@ -409,23 +402,40 @@ void ArduControl::tick()
        {
                Accessory &acc = *accessory_queue.front();
 
-               if(acc.state!=acc.target)
+               if(acc.state!=acc.target || acc.uncertain)
                {
-                       active_accessory = &acc;
-
                        unsigned changes = acc.state^acc.target;
                        unsigned lowest_bit = changes&~(changes-1);
-                       unsigned i;
-                       for(i=0; (lowest_bit>>i)>1; ++i) ;
-                       active_index = i;
-                       acc.state.set(acc.state^lowest_bit);
-                       PendingCommand cmd(acc, Accessory::ACTIVATE, i);
-                       command_queue.push(cmd);
-
-                       monitor.reset_peak();
+                       if(lowest_bit>>acc.bits)
+                       {
+                               // All remaining changes are in non-physical bits
+                               acc.state.set(acc.state^changes);
+                               acc.state.commit(acc.state.serial);
+                       }
+                       else
+                       {
+                               unsigned mask = (lowest_bit ? lowest_bit : acc.uncertain);
+                               for(active_index=0; (mask>>active_index)>1; ++active_index) ;
+                               acc.state.set(acc.state^lowest_bit);
+                               PendingCommand cmd(acc, Accessory::ACTIVATE, active_index);
+                               command_queue.push(cmd);
+                               active_accessory = &acc;
+
+                               monitor.reset_peak();
+                       }
                }
                else
+               {
                        accessory_queue.pop_front();
+
+                       if(acc.state==acc.target)
+                       {
+                               if(acc.kind==Accessory::TURNOUT)
+                                       signal_turnout.emit(acc.address, acc.state);
+                               else if(acc.kind==Accessory::SIGNAL)
+                                       signal_signal.emit(acc.address, acc.state);
+                       }
+               }
        }
 
        if(active_accessory && off_timeout)
@@ -435,17 +445,16 @@ void ArduControl::tick()
                {
                        Accessory &acc = *active_accessory;
 
-                       if(acc.kind==Accessory::TURNOUT && monitor.get_peak()<0.5f)
+                       unsigned bit = 1<<active_index;
+
+                       // Assume success if we were uncertain of the physical setting
+                       if(acc.uncertain&bit)
+                               acc.uncertain &= ~bit;
+                       else if(acc.kind==Accessory::TURNOUT && monitor.get_peak()<0.5f)
                        {
-                               unsigned bit = 1<<active_index;
-                               if(acc.uncertain&bit)
-                                       acc.uncertain &= ~bit;
-                               else
-                               {
-                                       signal_turnout_failed.emit(acc.address);
-                                       acc.state.rollback();
-                                       acc.target ^= bit;
-                               }
+                               signal_turnout_failed.emit(acc.address);
+                               acc.state.rollback();
+                               acc.target ^= bit;
                        }
 
                        off_timeout = Time::TimeStamp();