Telemetry from devices in Train view

[Steve Edmonds]

I have created issue #4269 in Githup with a fix that works, I have only worked in C ~20 years ago for machine control and suspect I have not defined my bool flag appropriately.

This fix seems to prevent seeing erroneous data in a workout when a device is not connected at power on but I will now see if I can improve the message "Connected.." when power on is pressed, which shows regardless of the actual devices, if any, in the device setup that actually connect. I.e. I have a device setup with a TurboMuin, cadence sensor and HR sensor and get the message "Connected.." regardless of which, if any of these actually connects.

[Steve Edmonds]

I have worked through Sean's guide and think I have created a patch for  #4269, not being that familiar with git and my first attempt at a patch.

Looking through this group, I take it that Sean's comment "join the Golden Cheetah Developer's Group and post a patch there" just involves posting the contents of the patch file as below.

From 603ad9e38db711047ce0e8d85555181fab3e625c Mon Sep 17 00:00:00 2001
From: Steve Edmonds
Date: Sat, 17 Sep 2022 15:32:44 +1200
Subject: [PATCH] Reset values for telemetry in Train view poweroff

Residual values for telemtery from devices can remain when powering off
after training. If that device does not nonnect in the next session,
i.e. HRM monitor not worn, that residual value from the previous session
is logged against the current session.
Exiting/Quitting GC and reloading it will clear the telemetry but this
is often inconvenient and easily forgotten.
fixes #4269
---
 src/Train/TrainSidebar.cpp | 28 ++++++++++++++++++++++++++++
 src/Train/TrainSidebar.h   |  1 +
 2 files changed, 29 insertions(+)

diff --git a/src/Train/TrainSidebar.cpp b/src/Train/TrainSidebar.cpp
index 2fbe39089..53796cba8 100644
--- a/src/Train/TrainSidebar.cpp
+++ b/src/Train/TrainSidebar.cpp
@@ -1619,6 +1619,7 @@ void TrainSidebar::Connect()
     gui_timer->start(REFRESHRATE);
 
     emit setNotification(tr("Connected.."), 2);
+    inDisconnect = false;
 }
 
 void TrainSidebar::Disconnect()
@@ -1645,7 +1646,11 @@ void TrainSidebar::Disconnect()
 
     gui_timer->stop();
 
+    inDisconnect = true;
+    guiUpdate();
+
     emit setNotification(tr("Disconnected.."), 2);
+    inDisconnect = false;
 }
 
 //----------------------------------------------------------------------
@@ -2019,6 +2024,29 @@ void TrainSidebar::guiUpdate()           // refreshes the telemetry
             //The system will be able to sleep again.
         }
 #endif
+    } else if (inDisconnect) {
+        foreach(int dev, activeDevices) {
+            if (dev == bpmTelemetry) rtData.setHr(0);
+            if (dev == rpmTelemetry) rtData.setCadence(0);
+            if (dev == kphTelemetry) {
+                    rtData.setSpeed(0.0);
+                    rtData.setDistance(0.0);
+                    rtData.setRouteDistance(0.0);
+                    rtData.setDistanceRemaining(0.0);
+                    rtData.setLapDistance(0.0);
+                    rtData.setLapDistanceRemaining(0.0);
+                }
+                if (dev == wattsTelemetry) {
+                    rtData.setWatts(0);
+                    rtData.setAltWatts(0);
+                    rtData.setLRBalance(0.0);
+                    rtData.setLTE(0);
+                    rtData.setRTE(0);
+                    rtData.setLPS(0);
+                    rtData.setRPS(0);
+                }
+       }
+       context->notifyTelemetryUpdate(rtData); // signal everyone to update telemetry
     }
 }
 
diff --git a/src/Train/TrainSidebar.h b/src/Train/TrainSidebar.h
index 07ad9c524..1484e4c06 100644
--- a/src/Train/TrainSidebar.h
+++ b/src/Train/TrainSidebar.h
@@ -317,6 +317,7 @@ class TrainSidebar : public GcWindow
         QSharedPointer<QFileSystemWatcher> watcher;
         bool calibrating;
         double wbalr, wbal;
+        bool inDisconnect = false;
 };
 
 class MultiDeviceDialog : public QDialog
--
2.35.3

[Ale Martinez]

This patch looks like a quick and dirty hack, it may be ok for your fork, but it cannot be merged to the repository.

I think to add this unrelated functionality to guiUpdate function is not a good solution, but doing that using a global flag instead of using a parameter, is really bad coding practice, IMHO.

[Steve Edmonds]

Thankyou Ale for the feedback, I am still feeling my way around the structure of the code and looking for small improvements I can make.

I considered the flag, a parameter and passing the name of the calling function (as available from C++20). I also agree it is not directly related to the guiUpdate function and considered for some time if calling the guiUpdate function to handle the case of NOT (status&RT_CONNECTED) was appropriate.

At present telemetry from physical devices associated with a defined train device does not appear to have consistent initialisation in the train view. From a functional point of view I considered the power on/off button to be a logical place to associate this activity. Any guidance in this respect would be much appreciated and would this be the correct forum to raise these issues.

[Ale Martinez]

El miércoles, 28 de septiembre de 2022 a la(s) 20:10:43 UTC-3, st...@edmondsfamily.co.nz escribió:

Thankyou Ale for the feedback, I am still feeling my way around the structure of the code and looking for small improvements I can make.

I considered the flag, a parameter and passing the name of the calling function (as available from C++20). I also agree it is not directly related to the guiUpdate function and considered for some time if calling the guiUpdate function to handle the case of NOT (status&RT_CONNECTED) was appropriate.

At present telemetry from physical devices associated with a defined train device does not appear to have consistent initialisation in the train view. From a functional point of view I considered the power on/off button to be a logical place to associate this activity. Any guidance in this respect would be much appreciated and would this be the correct forum to raise these issues.

As usual is all our interchanges, the afterwards justification is the worst part, perhaps it is better for me not to respond anymore.