https://telecominfraproject.atlassian.net/browse/WIFI-11388

Signed-off-by: stephb9959 <stephane.bourque@gmail.com>

build

@@ -1 +1 @@
-48
+49

issues/WIFI-11388.txt

@@ -0,0 +1,18 @@
+Issue: https://telecominfraproject.atlassian.net/browse/WIFI-11388
+
+Problem:
+If a configuration was accepted by the GW or Provisioning but is still not valid according to the firmware on teh device,
+the device will reject the configuration, however, that configuration is known as the kast good configuration in the GW.
+This mens that we will lock the device in a loop where it continuously wants to update the configuration to version X,
+and the device will continuously reject it.
+
+Workaround:
+Simply send a valid configuration to the GW and this will allow the device you update and stop the cycle.
+
+Fix:
+When a new configuration is submitted, store is a "pending". If it is accepted, move it to the current configuration. If
+not accepted, simply remove it. One corner case exists. For some configuration updates, the AP will never complete the
+update cycle, even if it has updated the configuration. In that case, we can detect the configuration during a connect
+later. At that moment, when we look for an upgrade, we must compare with the pending UUID and the current UUID. If it matches the pending,
+we know the last update worked. If it does not, we know to revert.
+

src/AP_WS_Connection.cpp

@@ -285,6 +285,13 @@ namespace OpenWifi {
 		GWObjects::Device D;
 		if (StorageService()->GetDevice(SerialNumber_, D)) {
 
+			if(D.pendingUUID!=0 && UUID==D.pendingUUID) {
+				//	so we sent an upgrade to a device, and now it is completing now...
+				UpgradedUUID = D.pendingUUID;
+				StorageService()->CompleteDeviceConfigurationChange(SerialNumber_);
+				return true;
+			}
+
 			//	This is the case where the cache is empty after a restart. So GoodConfig will 0. If
 			// the device already 	has the right UUID, we just return.
 			if (D.UUID == UUID) {
@@ -293,19 +300,24 @@ namespace OpenWifi {
 				return false;
 			}
 
+			Config::Config Cfg(D.Configuration);
 			if (UUID > D.UUID) {
 				//	so we have a problem, the device has a newer config than we have. So we need to
 				// make sure our config 	is newer.
-				Config::Config Cfg(D.Configuration);
 				D.UUID = UUID + 2;
 				UpgradedUUID = D.UUID;
 				Cfg.SetUUID(D.UUID);
 				D.Configuration = Cfg.get();
-				StorageService()->UpdateDevice(D);
+				StorageService()->SetPendingDeviceConfiguration(SerialNumber_,D.Configuration,D.UUID);
+			} else {
+				D.UUID=0;
+				StorageService()->SetPendingDeviceConfiguration(SerialNumber_,D.Configuration,D.UUID);
 			}
+			Cfg.SetUUID(D.UUID);
+			D.Configuration = Cfg.get();
+
+			State_.PendingUUID = UpgradedUUID = D.UUID;
 
-			UpgradedUUID = D.UUID;
-			State_.PendingUUID = D.UUID;
 			GWObjects::CommandDetails Cmd;
 			Cmd.SerialNumber = SerialNumber_;
 			Cmd.UUID = MicroServiceCreateUUID();

src/CommandManager.cpp

@@ -67,9 +67,12 @@ namespace OpenWifi {
 													   APCommands::to_string(RPC->second.Command)));
 								if (RPC->second.Command == APCommands::Commands::script) {
 									CompleteScriptCommand(RPC->second, Payload, rpc_execution_time);
-								} else if (RPC->second.Command != APCommands::Commands::telemetry) {
+								} else if (RPC->second.Command == APCommands::Commands::telemetry) {
 									CompleteTelemetryCommand(RPC->second, Payload,
 															 rpc_execution_time);
+								} else if (RPC->second.Command == APCommands::Commands::configure) {
+									CompleteConfigureCommand(RPC->second, Payload,
+															 rpc_execution_time);
 								} else {
 									StorageService()->CommandCompleted(RPC->second.UUID, Payload,
 																	   rpc_execution_time, true);
@@ -113,6 +116,40 @@ namespace OpenWifi {
 		return true;
 	}
 
+	bool CommandManager::CompleteConfigureCommand(
+		CommandInfo &Command, [[maybe_unused]] const Poco::JSON::Object::Ptr &Payload,
+		std::chrono::duration<double, std::milli> rpc_execution_time) {
+		std::shared_ptr<promise_type_t> TmpRpcEntry;
+
+		StorageService()->CommandCompleted(Command.UUID, Payload, rpc_execution_time, true);
+
+		if (Payload->has("result")) {
+			auto Result = Payload->getObject("result");
+			if (Result->has("status") && Result->has("serial")) {
+				auto Status = Result->getObject("status");
+				auto SerialNumber = Result->get("serial").toString();
+				std::uint64_t Error = Status->get("error");
+				if (Error == 2) {
+					StorageService()->RollbackDeviceConfigurationChange(SerialNumber);
+				} else {
+					StorageService()->CompleteDeviceConfigurationChange(SerialNumber);
+				}
+			}
+		} else {
+			//				std::cout << __LINE__ << std::endl;
+		}
+		Command.State = 0;
+
+		if (Command.rpc_entry) {
+			TmpRpcEntry = Command.rpc_entry;
+		}
+
+		OutStandingRequests_.erase(Command.Id);
+		if (TmpRpcEntry != nullptr)
+			TmpRpcEntry->set_value(Payload);
+		return true;
+	}
+
 	bool CommandManager::CompleteScriptCommand(
 		CommandInfo &Command, const Poco::JSON::Object::Ptr &Payload,
 		std::chrono::duration<double, std::milli> rpc_execution_time) {

src/CommandManager.h

@@ -188,6 +188,8 @@ namespace OpenWifi {
 								   std::chrono::duration<double, std::milli> rpc_execution_time);
 		bool CompleteTelemetryCommand(CommandInfo &Command, const Poco::JSON::Object::Ptr &Payload,
 									  std::chrono::duration<double, std::milli> rpc_execution_time);
+		bool CompleteConfigureCommand(CommandInfo &Command, const Poco::JSON::Object::Ptr &Payload,
+									  std::chrono::duration<double, std::milli> rpc_execution_time);
 
 		CommandManager() noexcept
 			: SubSystemServer("CommandManager", "CMD-MGR", "command.manager") {}

src/RESTAPI/RESTAPI_device_commandHandler.cpp

@@ -637,9 +637,8 @@ namespace OpenWifi {
 			}
 
 			auto When = GetWhen(Obj);
-			uint64_t NewUUID;
-
-			if (StorageService()->UpdateDeviceConfiguration(SerialNumber_, Configuration,
+			uint64_t NewUUID=0;
+			if (StorageService()->SetPendingDeviceConfiguration(SerialNumber_, Configuration,
 															NewUUID)) {
 				GWObjects::CommandDetails Cmd;
 
@@ -661,9 +660,21 @@ namespace OpenWifi {
 				Cmd.Details = ParamStream.str();
 
 				// AP_WS_Server()->SetPendingUUID(SerialNumber_, NewUUID);
-				return RESTAPI_RPC::WaitForCommand(CMD_RPC, APCommands::Commands::configure, true,
+				RESTAPI_RPC::WaitForCommand(CMD_RPC, APCommands::Commands::configure, true,
 												   Cmd, Params, *Request, *Response, timeout,
 												   nullptr, this, Logger_);
+
+				if(!Cmd.Executed) {
+					return;
+				}
+
+				if(Cmd.ErrorCode==2) {
+					StorageService()->RollbackDeviceConfigurationChange(SerialNumber_);
+				} else {
+					StorageService()->CompleteDeviceConfigurationChange(SerialNumber_);
+				}
+
+				return;
 			}
 			return BadRequest(RESTAPI::Errors::RecordNotUpdated);
 		}

src/StorageService.h

@@ -111,6 +111,11 @@ namespace OpenWifi {
 
 		bool UpdateDeviceConfiguration(std::string &SerialNumber, std::string &Configuration,
 									   uint64_t &NewUUID);
+		bool SetPendingDeviceConfiguration(std::string &SerialNumber, std::string &Configuration,
+									   uint64_t &NewUUID);
+
+		bool RollbackDeviceConfigurationChange(std::string & SerialNumber);
+		bool CompleteDeviceConfigurationChange(std::string & SerialNumber);
 
 		bool CreateDevice(GWObjects::Device &);
 		bool CreateDefaultDevice(std::string &SerialNumber, const Config::Capabilities &Caps,

src/storage/storage_device.cpp

@@ -237,6 +237,113 @@ namespace OpenWifi {
 		return false;
 	}
 
+	bool Storage::RollbackDeviceConfigurationChange(std::string & SerialNumber) {
+		try {
+			GWObjects::Device D;
+			if (!GetDevice(SerialNumber, D))
+				return false;
+			D.pendingConfiguration.clear();
+			D.pendingUUID = 0;
+			D.LastConfigurationChange = Utils::Now();
+
+			ConfigurationCache().Add(Utils::SerialNumberToInt(SerialNumber), D.UUID);
+
+			Poco::Data::Session Sess = Pool_->get();
+			Poco::Data::Statement Update(Sess);
+
+			DeviceRecordTuple R;
+			ConvertDeviceRecord(D, R);
+			std::string St2{"UPDATE Devices SET " + DB_DeviceUpdateFields +
+							" WHERE SerialNumber=?"};
+			Update << ConvertParams(St2), Poco::Data::Keywords::use(R),
+				Poco::Data::Keywords::use(SerialNumber);
+			Update.execute();
+			return true;
+		} catch (const Poco::Exception &E) {
+			Logger().log(E);
+		}
+		return false;
+	}
+
+	bool Storage::CompleteDeviceConfigurationChange(std::string & SerialNumber) {
+		try {
+			GWObjects::Device D;
+			if (!GetDevice(SerialNumber, D))
+				return false;
+			D.Configuration = D.pendingConfiguration;
+			D.pendingConfiguration.clear();
+			D.UUID = D.pendingUUID;
+			D.pendingUUID = 0;
+			D.LastConfigurationChange = Utils::Now();
+
+			ConfigurationCache().Add(Utils::SerialNumberToInt(SerialNumber), D.UUID);
+
+			Poco::Data::Session Sess = Pool_->get();
+			Poco::Data::Statement Update(Sess);
+
+			DeviceRecordTuple R;
+			ConvertDeviceRecord(D, R);
+			std::string St2{"UPDATE Devices SET " + DB_DeviceUpdateFields +
+							" WHERE SerialNumber=?"};
+			Update << ConvertParams(St2), Poco::Data::Keywords::use(R),
+				Poco::Data::Keywords::use(SerialNumber);
+			Update.execute();
+			return true;
+		} catch (const Poco::Exception &E) {
+			Logger().log(E);
+		}
+		return false;
+	}
+
+	bool Storage::SetPendingDeviceConfiguration(std::string &SerialNumber, std::string &Configuration,
+									   uint64_t &NewUUID) {
+		try {
+
+			Config::Config Cfg(Configuration);
+			if (!Cfg.Valid()) {
+				poco_warning(Logger(), fmt::format("CONFIG-UPDATE({}): Configuration was not valid",
+												   SerialNumber));
+				return false;
+			}
+
+			Poco::Data::Session Sess = Pool_->get();
+			Poco::Data::Statement Select(Sess);
+
+			GWObjects::Device D;
+			if (!GetDevice(SerialNumber, D))
+				return false;
+
+			uint64_t Now = time(nullptr);
+			if(NewUUID==0) {
+				D.pendingUUID = NewUUID = (D.LastConfigurationChange == Now ? Now + 1 : Now);
+			} else {
+				D.pendingUUID = NewUUID;
+			}
+
+			if (Cfg.SetUUID(NewUUID)) {
+				Poco::Data::Statement Update(Sess);
+				D.pendingConfiguration = Cfg.get();
+
+				DeviceRecordTuple R;
+				ConvertDeviceRecord(D, R);
+				std::string St2{"UPDATE Devices SET " + DB_DeviceUpdateFields +
+								" WHERE SerialNumber=?"};
+				Update << ConvertParams(St2), Poco::Data::Keywords::use(R),
+					Poco::Data::Keywords::use(SerialNumber);
+				Update.execute();
+				poco_information(Logger(),
+								 fmt::format("DEVICE-PENDING-CONFIGURATION-UPDATED({}): New UUID is {}",
+											 SerialNumber, NewUUID));
+				Configuration = D.Configuration;
+				return true;
+			}
+			return false;
+		} catch (const Poco::Exception &E) {
+			Logger().log(E);
+		}
+		return false;
+	}
+
 	bool Storage::CreateDevice(GWObjects::Device &DeviceDetails) {
 		std::string SerialNumber;
 		try {