From b79fbe6985cb9bcd8df1f5b0ec95b25314d8798d Mon Sep 17 00:00:00 2001
From: Mikko Rasa <tdb@tdb.fi>
Date: Sun, 6 Apr 2014 23:08:32 +0300
Subject: [PATCH] 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.
---
 source/libr2c2/arducontrol.cpp | 67 +++++++++++++++++++---------------
 1 file changed, 38 insertions(+), 29 deletions(-)

diff --git a/source/libr2c2/arducontrol.cpp b/source/libr2c2/arducontrol.cpp
index 98f7433..2cdf0c6 100644
--- 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();
-- 
2.43.0