Move changed check and added listener to ManagedDataObjectBase. Also …

…change some ctor parameters to use move

fbw-common/src/wasm/extra-backend/MsfsHandler/DataManager/AircraftVariable.cpp

@@ -97,7 +97,7 @@ std::string AircraftVariable::str() const {
   ss << "AircraftVariable: [" << name << (index ? ":" + std::to_string(index) : "");
   ss << ", value: " << (cachedValue.has_value() ? std::to_string(cachedValue.value()) : "N/A");
   ss << ", unit: " << unit.name;
-  ss << ", changed: " << changed;
+  ss << ", changed: " << hasChanged();
   ss << ", dirty: " << dirty;
   ss << ", timeStamp: " << timeStampSimTime;
   ss << ", tickStamp: " << tickStamp;

fbw-common/src/wasm/extra-backend/MsfsHandler/DataManager/AircraftVariable.h

@@ -59,15 +59,15 @@ class AircraftVariable : public CacheableVariable {
    * @param setterEventName The calculator code to write to the variable.
    */
   explicit AircraftVariable(
-    const std::string &varName,
+    const std::string varName,
     int varIndex = 0,
     std::string setterEventName = "",
     Unit unit = UNITS.Number,
     bool autoReading = false,
     bool autoWriting = false,
     FLOAT64 maxAgeTime = 0.0,
     UINT64 maxAgeTicks = 0)
-    : CacheableVariable(varName, unit, autoReading, autoWriting, maxAgeTime, maxAgeTicks),
+    : CacheableVariable(std::move(varName), unit, autoReading, autoWriting, maxAgeTime, maxAgeTicks),
       index(varIndex), setterEventName(std::move(setterEventName)), setterEvent(nullptr) {
 
     dataID = get_aircraft_var_enum(varName.c_str());

fbw-common/src/wasm/extra-backend/MsfsHandler/DataManager/CacheableVariable.cpp

@@ -21,17 +21,14 @@ FLOAT64 CacheableVariable::get() const {
 
 FLOAT64 CacheableVariable::updateFromSim(FLOAT64 timeStamp, UINT64 tickCounter) {
   if (cachedValue.has_value() && !needsUpdateFromSim(timeStamp, tickCounter)) {
-    changed = false;
+    setChanged(false);
     LOG_TRACE("CacheableVariable::updateFromSim() - from cache "
               + this->name
               + " " + str()
     );
     return cachedValue.value();
   }
-  LOG_TRACE("CacheableVariable::updateFromSim() - read from sim "
-            + this->name
-            + " " + str()
-  );
+  LOG_TRACE("CacheableVariable::updateFromSim() - read from sim " + this->name + " " + str());
   // update the value from the sim
   timeStampSimTime = timeStamp;
   tickStamp = tickCounter;
@@ -40,27 +37,20 @@ FLOAT64 CacheableVariable::updateFromSim(FLOAT64 timeStamp, UINT64 tickCounter)
 
 FLOAT64 CacheableVariable::readFromSim() {
   const FLOAT64 fromSim = rawReadFromSim();
-
   // compare the value from the sim with the cached value
-  changed = !cachedValue.has_value()
+  bool changed = skipChangeCheck || !cachedValue.has_value()
             || !helper::Math::almostEqual(fromSim, cachedValue.value(), epsilon);
-
-  // Handling of "changed" - two options
-  // 1. new field to remember the last value marked as changed and compare it to the new value
-  // 2. do not update the cache value and discard the sim read (which is a bit of waste)
-  // Option 2 has been chosen for now as it is simpler and doesn't need the extra field.
-  if (changed) {
-    cachedValue = fromSim;
-  }
-
+  if (changed) cachedValue = fromSim;
   dirty = false;
+  setChanged(changed);
   return cachedValue.value();
 }
 
 void CacheableVariable::set(FLOAT64 value) {
   if (cachedValue.has_value() && cachedValue.value() == value) {
     return;
   }
+  // TODO: should hasChanged be set to true here? Would call all subscribers' callbacks
   cachedValue = value;
   dirty = true;
 }
@@ -79,7 +69,6 @@ void CacheableVariable::setAndWriteToSim(FLOAT64 value) {
 
 void CacheableVariable::writeToSim() {
   if (cachedValue.has_value()) {
-    changed = false;
     dirty = false;
     rawWriteToSim();
     return;

fbw-common/src/wasm/extra-backend/MsfsHandler/DataManager/CacheableVariable.h

@@ -49,11 +49,6 @@ class CacheableVariable : public ManagedDataObjectBase {
    */
   bool dirty = false;
 
-  /**
-   * Flag to indicate if the variable has changed compared to the last read/write from the sim.
-   */
-  bool changed = false;
-
   /**
    * The epsilon required to change a variable after a read from the sim. This is used to
    * set the changed flag and cache the new value if it is different by >epsilon from the last
@@ -80,13 +75,13 @@ class CacheableVariable : public ManagedDataObjectBase {
    * @param maxAgeTicks The maximum age of the variable in ticks when using updateDataToSim()
    */
   CacheableVariable(
-    const std::string &varName,
+    const std::string varName,
     const Unit &unit,
     bool autoRead,
     bool autoWrite,
     FLOAT64 maxAgeTime,
     UINT64 maxAgeTicks)
-    : ManagedDataObjectBase(varName, autoRead, autoWrite, maxAgeTime, maxAgeTicks), unit(unit) {}
+    : ManagedDataObjectBase(std::move(varName), autoRead, autoWrite, maxAgeTime, maxAgeTicks), unit(unit) {}
 
 public:
   CacheableVariable() = delete; // no default constructor
@@ -225,12 +220,6 @@ class CacheableVariable : public ManagedDataObjectBase {
    */
   void setAsInt64(UINT64 i) { set(static_cast<FLOAT64>(i)); }
 
-  /**
-   * @return true if the value has changed since the last read from the sim.
-   */
-  [[nodiscard]]
-  bool hasChanged() const { return changed; }
-
   /**
    * @return Epsilon used for comparing floating point values. Variables are considered equal if the
    * difference is smaller than this value.

fbw-common/src/wasm/extra-backend/MsfsHandler/DataManager/ClientDataAreaVariable.hpp

@@ -47,7 +47,7 @@
  */
 template<typename T>
 class ClientDataAreaVariable : public SimObjectBase {
-private:
+protected:
   // The client data area ID
   SIMCONNECT_CLIENT_DATA_ID clientDataId;
 
@@ -74,22 +74,28 @@ class ClientDataAreaVariable : public SimObjectBase {
    * @param clientDataDefinitionId A unique ID for the client data definition, specified by the client.
    *                               This class only supports one definition per client data area,
    *                               the definition given by the template parameter type
-   * @param requestId Each request for sim object data requires a unique id so the sim can provide the request ID in the response (message
-   *                  SIMCONNECT_RECV_ID_SIMOBJECT_DATA).
-   * @param autoReading Used by external classes to determine if the variable should updated from the sim when a sim update call occurs.
-   * @param autoWriting Used by external classes to determine if the variable should written to the sim when a sim update call occurs.
-   * @param maxAgeTime The maximum age of the value in sim time before it is updated from the sim by the requestUpdateFromSim() method.
-   * @param maxAgeTicks The maximum age of the value in ticks before it is updated from the sim by the requestUpdateFromSim() method.
+   * @param requestId Each request for sim object data requires a unique id so the sim can provide
+*                     the request ID in the response (message SIMCONNECT_RECV_ID_SIMOBJECT_DATA).
+   * @param autoReading Used by external classes to determine if the variable should updated from
+   *                    the sim when a sim update call occurs.
+   * @param autoWriting Used by external classes to determine if the variable should written to the
+ *                      sim when a sim update call occurs.
+   * @param maxAgeTime The maximum age of the value in sim time before it is updated from the sim by
+   *                   the requestUpdateFromSim() method.
+   * @param maxAgeTicks The maximum age of the value in ticks before it is updated from the sim by
+   *                    the requestUpdateFromSim() method.
    */
-  ClientDataAreaVariable<T>(HANDLE hSimConnect, const std::string &clientDataName,
-                            SIMCONNECT_CLIENT_DATA_ID clientDataId,
-                            SIMCONNECT_CLIENT_DATA_DEFINITION_ID clientDataDefinitionId,
-                            SIMCONNECT_DATA_REQUEST_ID requestId,
-                            bool autoRead = false,
-                            bool autoWrite = false,
-                            FLOAT64 maxAgeTime = 0.0,
-                            UINT64 maxAgeTicks = 0)
-    : SimObjectBase(hSimConnect, clientDataName, clientDataDefinitionId, requestId,
+  ClientDataAreaVariable<T>(
+    HANDLE hSimConnect,
+    const std::string clientDataName,
+    SIMCONNECT_CLIENT_DATA_ID clientDataId,
+    SIMCONNECT_CLIENT_DATA_DEFINITION_ID clientDataDefinitionId,
+    SIMCONNECT_DATA_REQUEST_ID requestId,
+    bool autoRead = false,
+    bool autoWrite = false,
+    FLOAT64 maxAgeTime = 0.0,
+    UINT64 maxAgeTicks = 0)
+    : SimObjectBase(hSimConnect, std::move(clientDataName), clientDataDefinitionId, requestId,
                     autoRead, autoWrite, maxAgeTime, maxAgeTicks),
       clientDataId(clientDataId) {
 
@@ -113,8 +119,6 @@ class ClientDataAreaVariable : public SimObjectBase {
       hSimConnect, clientDataDefinitionId, SIMCONNECT_CLIENTDATAOFFSET_AUTO, sizeof(T)))) {
       LOG_ERROR("ClientDataAreaVariable: Adding to client data definition failed: " + name);
     }
-
-    setDataChanged(false);
   }
 
   /**
@@ -149,8 +153,11 @@ class ClientDataAreaVariable : public SimObjectBase {
   }
 
   [[nodiscard]] bool requestDataFromSim() const override {
-    if (!SUCCEEDED(SimConnect_RequestClientData(hSimConnect, clientDataId, requestId, dataDefId, SIMCONNECT_CLIENT_DATA_PERIOD_ONCE,
-                                                SIMCONNECT_CLIENT_DATA_REQUEST_FLAG_DEFAULT))) {
+    if (!SUCCEEDED(SimConnect_RequestClientData(
+      hSimConnect, clientDataId, requestId, dataDefId,
+      SIMCONNECT_CLIENT_DATA_PERIOD_ONCE,
+      SIMCONNECT_CLIENT_DATA_REQUEST_FLAG_DEFAULT))
+      ) {
       LOG_ERROR("ClientDataAreaVariable: Requesting client data failed: " + name);
       return false;
     }
@@ -181,11 +188,11 @@ class ClientDataAreaVariable : public SimObjectBase {
    */
   [[nodiscard]] bool requestPeriodicDataFromSim(
     SIMCONNECT_CLIENT_DATA_PERIOD period,
-    SIMCONNECT_CLIENT_DATA_REQUEST_FLAG  periodFlags = SIMCONNECT_CLIENT_DATA_REQUEST_FLAG_DEFAULT,
+    SIMCONNECT_CLIENT_DATA_REQUEST_FLAG periodFlags = SIMCONNECT_CLIENT_DATA_REQUEST_FLAG_DEFAULT,
     DWORD origin = 0,
     DWORD interval = 0,
     DWORD limit = 0
-    ) const {
+  ) const {
     if (autoRead && period >= SIMCONNECT_CLIENT_DATA_PERIOD_ONCE) {
       LOG_ERROR("ClientDataAreaVariable: Requested periodic data update from sim is ignored as autoRead is enabled.");
       return false;
@@ -218,60 +225,62 @@ class ClientDataAreaVariable : public SimObjectBase {
   }
 
   void processSimData(const SIMCONNECT_RECV* pData, FLOAT64 simTime, UINT64 tickCounter) override {
-    LOG_INFO("ClientDataAreaVariable: Received client data: " + name);
+    LOG_TRACE("ClientDataAreaVariable: Received client data: " + name);
     const auto pClientData = reinterpret_cast<const SIMCONNECT_RECV_CLIENT_DATA*>(pData);
-    SIMPLE_ASSERT(pClientData->dwRequestID == requestId,
-                  "DataDefinitionVariable::processSimData: Request ID mismatch")
+
+   SIMPLE_ASSERT(pClientData->dwRequestID == requestId,
+                  "ClientDataAreaVariable::processSimData: Request ID mismatch: " + name);
+
     // if not required then skip the rather expensive check for change
-    dataChanged = skipChangeCheck || std::memcmp(&pClientData->dwData, &this->dataStruct, sizeof(T)) != 0;
-    if (dataChanged) {
+    if (skipChangeCheck || std::memcmp(&pClientData->dwData, &this->dataStruct, sizeof(T)) != 0) {
       LOG_TRACE("ClientDataAreaVariable: Data has changed: " + name);
       std::memcpy(&this->dataStruct, &pClientData->dwData, sizeof(T));
       timeStampSimTime = simTime;
       tickStamp = tickCounter;
+      setChanged(true);
       return;
     }
+    setChanged(false);
     LOG_TRACE("ClientDataAreaVariable: Data has not changed: " + name);
   }
 
   bool writeDataToSim() override {
     if (!SUCCEEDED(SimConnect_SetClientData(hSimConnect, clientDataId, dataDefId,
                                             SIMCONNECT_CLIENT_DATA_SET_FLAG_DEFAULT,
                                             0, sizeof(T), &this->dataStruct))) {
-      LOG_ERROR("DataDefinitionVariable: Setting data to sim for " + name
+      LOG_ERROR("ClientDataAreaVariable: Setting data to sim for " + name
                 + " with dataDefId=" + std::to_string(dataDefId) + " failed!");
       return false;
     }
-    LOG_TRACE("DataDefinitionVariable: Setting data to sim for " + name
+    LOG_TRACE("ClientDataAreaVariable: Setting data to sim for " + name
               + " with dataDefId=" + std::to_string(dataDefId) + " succeeded.");
-    setDataChanged(false);
     return true;
   }
 
   /**
    * Returns a modifiable reference to the data container
    * @return T& Reference to the data container
    */
-  [[maybe_unused]] [[nodiscard]] T &data() { return dataStruct; }
+  T &data() { return dataStruct; }
 
   /**
    * Returns a constant reference to the data container
    * @return std::vector<T>& Reference to the data container
    */
-  [[maybe_unused]] [[nodiscard]] const T &data() const { return dataStruct; }
+  const T &data() const { return dataStruct; }
 
   [[nodiscard]] std::string str() const
   override {
     std::stringstream ss;
-    ss << "DataDefinition[ name=" << getName();
+    ss << "ClientDataAreaVariable[ name=" << getName();
     ss << ", clientDataId=" << clientDataId;
     ss << ", dataDefId=" << dataDefId;
     ss << ", requestId=" << requestId;
     ss << ", structSize=" << sizeof(T);
     ss << ", timeStamp: " << timeStampSimTime;
     ss << ", tickStamp: " << tickStamp;
     ss << ", skipChangeCheck: " << skipChangeCheck;
-    ss << ", dataChanged: " << dataChanged;
+    ss << ", dataChanged: " << hasChanged();
     ss << ", autoRead: " << autoRead;
     ss << ", autoWrite: " << autoWrite;
     ss << ", maxAgeTime: " << maxAgeTime;

fbw-common/src/wasm/extra-backend/MsfsHandler/DataManager/DataDefinitionVariable.hpp

@@ -99,7 +99,7 @@ class DataDefinitionVariable : public SimObjectBase {
    */
   DataDefinitionVariable<T>(
     HANDLE hSimConnect,
-    const std::string &varName,
+    const std::string varName,
     const std::vector<DataDefinition> &dataDefinitions,
     SIMCONNECT_DATA_DEFINITION_ID dataDefId,
     SIMCONNECT_DATA_REQUEST_ID requestId,
@@ -108,12 +108,9 @@ class DataDefinitionVariable : public SimObjectBase {
     FLOAT64 maxAgeTime = 0.0,
     UINT64 maxAgeTicks = 0
   )
-    : SimObjectBase(hSimConnect, varName, dataDefId, requestId, autoRead, autoWrite, maxAgeTime, maxAgeTicks),
+    : SimObjectBase(hSimConnect, std::move(varName), dataDefId, requestId, autoRead, autoWrite, maxAgeTime, maxAgeTicks),
       dataDefinitions(dataDefinitions), dataStruct{} {
 
-    SIMPLE_ASSERT(sizeof(T) == dataDefinitions.size() * sizeof(FLOAT64),
-                  "DataDefinitionVariable::processSimData: Struct size mismatch")
-
     for (auto &ddef: dataDefinitions) {
       std::string fullVarName = ddef.name;
       if (ddef.index != 0) fullVarName += ":" + std::to_string(ddef.index);
@@ -148,8 +145,8 @@ class DataDefinitionVariable : public SimObjectBase {
       requestId,
       dataDefId,
       SIMCONNECT_OBJECT_ID_USER,
-      SIMCONNECT_PERIOD_ONCE))) {
-
+      SIMCONNECT_PERIOD_ONCE))
+      ) {
       LOG_ERROR("DataDefinitionVariable: Failed to request data from sim: " + name);
       return false;
     }
@@ -216,19 +213,20 @@ class DataDefinitionVariable : public SimObjectBase {
   void processSimData(const SIMCONNECT_RECV* pData, FLOAT64 simTime, UINT64 tickCounter) override {
     LOG_TRACE("DataDefinitionVariable: Received client data: " + name);
     const auto pSimobjectData = reinterpret_cast<const SIMCONNECT_RECV_SIMOBJECT_DATA*>(pData);
-    SIMPLE_ASSERT(sizeof(T) == pSimobjectData->dwDefineCount * sizeof(FLOAT64),
-                  "DataDefinitionVariable::processSimData: Struct size mismatch")
-    SIMPLE_ASSERT(pSimobjectData->dwRequestID == requestId,
-                  "DataDefinitionVariable::processSimData: Request ID mismatch")
+
+   SIMPLE_ASSERT(pSimobjectData->dwRequestID == requestId,
+           "DataDefinitionVariable::processSimData: Request ID mismatch: " + name);
+
     // if not required then skip the rather expensive check for change
-    dataChanged = skipChangeCheck || std::memcmp(&pSimobjectData->dwData, &this->dataStruct, sizeof(T)) != 0;
-    if (dataChanged) {
+    if (skipChangeCheck || std::memcmp(&pSimobjectData->dwData, &this->dataStruct, sizeof(T)) != 0) {
       LOG_TRACE("DataDefinitionVariable: Data has changed: " + name);
       std::memcpy(&this->dataStruct, &pSimobjectData->dwData, sizeof(T));
       timeStampSimTime = simTime;
       tickStamp = tickCounter;
+      setChanged(true);
       return;
     }
+    setChanged(false);
     LOG_TRACE("DataDefinitionVariable: Data has not changed: " + name);
   };
 
@@ -240,7 +238,6 @@ class DataDefinitionVariable : public SimObjectBase {
                 + std::to_string(dataDefId) + " failed!");
       return false;
     }
-    setDataChanged(false);
     return true;
   };
 
@@ -277,7 +274,7 @@ class DataDefinitionVariable : public SimObjectBase {
     ss << ", timeStamp: " << timeStampSimTime;
     ss << ", tickStamp: " << tickStamp;
     ss << ", skipChangeCheck: " << skipChangeCheck;
-    ss << ", dataChanged: " << dataChanged;
+    ss << ", dataChanged: " << hasChanged();
     ss << ", autoRead: " << autoRead;
     ss << ", autoWrite: " << autoWrite;
     ss << ", maxAgeTime: " << maxAgeTime;