Skip to content

Commit 29415ea

Browse files
committed
Announce the simulated bike after homeform exists, not before
Installed on Android, the simulated bike came up with no tiles. The log said why, and said it precisely: 219 metric points at C: 85 W: 150 R: 12 - the built-in ride, running exactly as written - a live DIRCON endpoint, homeform alive throughout, and not one call to homeform::deviceConnected(), which logs on entry and so leaves no doubt. A real device is discovered from a radio callback, which by definition happens once the event loop is running, and therefore after main() has built homeform and connected it to deviceConnected() and bluetoothDeviceConnected(). The simulated bike is built in the bluetooth constructor instead - about a hundred lines of main() earlier - so announcing it there announced it to an empty connection list. deviceConnected() was never emitted at all; the ftmsbike path emits it from deviceDiscovered() and the new path had no equivalent. That function is what clears the help label and builds the session, so without it the app has a working bike and no dashboard. The object is still built in the constructor, because homeform::deviceConnected() returns early on a null device and being late with it would break the same thing from the other end. Only the announcement moves, onto a zero timer that fires on the first turn of the event loop. The announced QBluetoothDeviceInfo carries the scenario's `bike` name, or "Simulated Bike", so homeform::deviceFound() has something to show. Second bug, same install: firstRun() did not know about simulated_bike, so with nothing ever discovered it concluded QZ had never been set up and Home.qml pushed the wizard over the dashboard. That condition still names the dead applewatch_fakedevice flag for exactly this reason - suppressing the wizard was the only surviving effect that flag had, which this fork's own spec had recorded and this change had then failed to carry across. Three tests pin it, and were checked the only way a regression test is worth anything: by reverting the fix and watching them fail with "homeform would never build the session" and "the template managers would never start". 189 tests pass. They also cost a lesson worth leaving in the file: TestSettings does not activate itself, and an inactive one writes to its own file while the code under test reads the default - so the first version of these tests failed in a way that looked like a bug in the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019d4Hga7UGcxmcG88pwcSCV
1 parent 116f7bf commit 29415ea

5 files changed

Lines changed: 160 additions & 3 deletions

File tree

src/devices/bluetooth.cpp

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,33 @@ bluetooth::bluetooth(bool logs, const QString &deviceName, bool noWriteResistanc
8585
connect(simulatedBike, &bluetoothdevice::connectedAndDiscovered, this,
8686
&bluetooth::connectedAndDiscovered);
8787
connect(simulatedBike, &simulatedbike::debug, this, &bluetooth::debug);
88-
this->signalBluetoothDeviceConnected(simulatedBike);
88+
89+
// The device exists from here - device() must not return null while the rest of
90+
// startup runs - but announcing it here would announce it to nobody. This
91+
// constructor runs from main() around a hundred lines before homeform is built,
92+
// and homeform is what connects to deviceConnected and bluetoothDeviceConnected.
93+
// A real device is discovered from a radio callback, which by definition happens
94+
// after the event loop is running and therefore after homeform exists; a device
95+
// built during construction has no such luck, and both signals went into the
96+
// void. The visible result was an app with no tiles: homeform::deviceConnected()
97+
// is what clears the help label and builds the session, and it never ran.
98+
//
99+
// A zero timer is the whole fix: it fires on the first turn of the event loop,
100+
// which main() does not reach until homeform is constructed and connected.
101+
QTimer::singleShot(0, this, [this]() {
102+
if (!simulatedBike)
103+
return;
104+
// homeform calls deviceFound() with this name, so the scenario's `bike`
105+
// directive is what the user sees it identify as.
106+
QString name = QString::fromStdString(simulatedBike->scenario().bike());
107+
if (name.isEmpty())
108+
name = QStringLiteral("Simulated Bike");
109+
QBluetoothDeviceInfo info(QBluetoothAddress(quint64(1)), name, 0);
110+
info.setCoreConfigurations(QBluetoothDeviceInfo::LowEnergyCoreConfiguration);
111+
emit deviceConnected(info);
112+
this->signalBluetoothDeviceConnected(simulatedBike);
113+
});
114+
89115
this->discoveryAgent = nullptr;
90116
return;
91117
}

src/homeform.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,8 +419,15 @@ class homeform : public QObject {
419419
settings.value(QZSettings::fakedevice_treadmill, QZSettings::default_fakedevice_treadmill).toBool();
420420
bool antbike =
421421
settings.value(QZSettings::antbike, QZSettings::default_antbike).toBool();
422-
423-
return settings.value(QZSettings::bluetooth_lastdevice_name, QZSettings::default_bluetooth_lastdevice_name).toString().isEmpty() &&
422+
// A simulated bike is a configured device. Without this the wizard opens over the
423+
// dashboard on every start: nothing was ever discovered, so bluetooth_lastdevice_name
424+
// is empty and QZ concludes it has never been set up. This is the same reason the
425+
// dead applewatch_fakedevice flag is still named in this condition.
426+
bool simulated_bike =
427+
settings.value(QZSettings::simulated_bike, QZSettings::default_simulated_bike).toBool();
428+
429+
return settings.value(QZSettings::bluetooth_lastdevice_name, QZSettings::default_bluetooth_lastdevice_name).toString().isEmpty() &&
430+
!simulated_bike &&
424431
nordictrack_2950_ip.isEmpty() && tdf_10_ip.isEmpty() && !fake_bike && !fakedevice_elliptical &&
425432
!fakedevice_rower && !waterrower_usb && !fakedevice_treadmill && !antbike && !android_antbike && proform_elliptical_ip.isEmpty() &&
426433
proformtdf4ip.isEmpty() && proformtdf1ip.isEmpty() && proformtreadmillip.isEmpty() &&
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
#include "TestSimulatedBikeAnnouncement.h"
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
#pragma once
2+
3+
#include <gtest/gtest.h>
4+
5+
#include <QBluetoothDeviceInfo>
6+
#include <QCoreApplication>
7+
#include <QDateTime>
8+
#include <QTimer>
9+
10+
#include "Tools/testsettings.h"
11+
#include "devices/bluetooth.h"
12+
#include "devices/simulatedbike/simulatedbike.h"
13+
#include "qzsettings.h"
14+
15+
namespace {
16+
17+
/**
18+
* The bug this pins, in full, because it cost an install and a log to find and would be
19+
* invisible to every other test here.
20+
*
21+
* A real device is discovered from a radio callback. That happens once the event loop is
22+
* running, which is long after main() has built homeform and connected it to the two signals
23+
* that matter - deviceConnected(), which builds the session and clears the help label, and
24+
* bluetoothDeviceConnected(), which starts the template managers.
25+
*
26+
* The simulated bike is built in the bluetooth constructor instead, roughly a hundred lines
27+
* of main() before homeform exists. Announcing it there announced it to nobody: both signals
28+
* were emitted into an empty connection list, deviceConnected() was never emitted at all, and
29+
* the app came up with a working bike, a live DIRCON endpoint, a full metric stream - and no
30+
* tiles. The device log said it plainly: 219 metric points, and not one call to
31+
* homeform::deviceConnected().
32+
*
33+
* So the announcement is deferred to a zero timer, and this test is the thing that says so.
34+
* It asserts what the constructor cannot: that by the time the event loop has turned once,
35+
* the signals homeform needs have actually been emitted.
36+
*/
37+
class SimulatedBikeAnnouncement : public ::testing::Test {
38+
protected:
39+
TestSettings testSettings{"Roberto Viola", "QZ Simulated Bike Test"};
40+
41+
void SetUp() override {
42+
// TestSettings does not activate itself, and an inactive one is a silent no-op: the
43+
// values land in its own file while the code under test goes on reading the default
44+
// one. The first version of this test failed for exactly that reason and looked like
45+
// a bug in the fix it was written to prove.
46+
testSettings.activate();
47+
48+
// Nothing here may touch a radio or a socket: this runs on a CI box with neither.
49+
testSettings.qsettings.setValue(QZSettings::virtual_device_enabled, false);
50+
testSettings.qsettings.setValue(QZSettings::virtual_device_bluetooth, false);
51+
testSettings.qsettings.setValue(QZSettings::dircon_yes, false);
52+
testSettings.qsettings.setValue(QZSettings::simulated_bike, true);
53+
testSettings.qsettings.setValue(QZSettings::simulated_bike_ride, QLatin1String(""));
54+
}
55+
56+
/**
57+
* Turn the event loop until the flag is set, or the deadline passes. Deliberately not
58+
* QSignalSpy: that lives in Qt's testlib, and this project does not link it - adding a Qt
59+
* module so one test can count signals would be paid for by every platform's build.
60+
*/
61+
static void spinUntil(const bool &flag, int ms = 2000) {
62+
const qint64 deadline = QDateTime::currentMSecsSinceEpoch() + ms;
63+
while (!flag && QDateTime::currentMSecsSinceEpoch() < deadline)
64+
QCoreApplication::processEvents(QEventLoop::AllEvents, 50);
65+
}
66+
};
67+
68+
TEST_F(SimulatedBikeAnnouncement, AnnouncesTheDeviceOnceTheEventLoopRuns) {
69+
bluetooth bt(false);
70+
int deviceConnected = 0;
71+
int bikeConnected = 0;
72+
QObject::connect(&bt, &bluetooth::deviceConnected,
73+
[&deviceConnected](QBluetoothDeviceInfo) { ++deviceConnected; });
74+
QObject::connect(&bt, &bluetooth::bluetoothDeviceConnected,
75+
[&bikeConnected](bluetoothdevice *) { ++bikeConnected; });
76+
77+
// The device exists immediately - homeform::deviceConnected() returns early on a null
78+
// device, so being late with the object would break it just as thoroughly as being early
79+
// with the signal.
80+
ASSERT_NE(nullptr, bt.device());
81+
EXPECT_NE(nullptr, dynamic_cast<simulatedbike *>(bt.device()));
82+
83+
// ...but nothing has been announced yet, which is the whole point: a listener connected
84+
// after this constructor returns must still hear about it.
85+
EXPECT_EQ(0, deviceConnected);
86+
EXPECT_EQ(0, bikeConnected);
87+
88+
bool announced = false;
89+
QObject::connect(&bt, &bluetooth::deviceConnected, [&announced](QBluetoothDeviceInfo) { announced = true; });
90+
spinUntil(announced);
91+
92+
EXPECT_EQ(1, deviceConnected) << "homeform would never build the session";
93+
EXPECT_EQ(1, bikeConnected) << "the template managers would never start";
94+
}
95+
96+
TEST_F(SimulatedBikeAnnouncement, TheAnnouncedDeviceCarriesAUsableName) {
97+
bluetooth bt(false);
98+
bool announced = false;
99+
QBluetoothDeviceInfo info;
100+
QObject::connect(&bt, &bluetooth::deviceConnected, [&](QBluetoothDeviceInfo i) {
101+
info = i;
102+
announced = true;
103+
});
104+
spinUntil(announced);
105+
106+
ASSERT_TRUE(announced);
107+
// homeform passes this to deviceFound(), so an empty name is a blank device label.
108+
EXPECT_FALSE(info.name().isEmpty());
109+
EXPECT_TRUE(info.isValid());
110+
}
111+
112+
TEST_F(SimulatedBikeAnnouncement, NoSimulatedBikeWhenTheSettingIsOff) {
113+
testSettings.qsettings.setValue(QZSettings::simulated_bike, false);
114+
115+
// startDiscovery false, so this builds nothing and starts no scan - the point is only
116+
// that the simulated bike is not conjured up when it was not asked for.
117+
bluetooth bt(true, QLatin1String(""), false, false, 200, false, false, 4, 1.0, false);
118+
EXPECT_EQ(nullptr, bt.device());
119+
}
120+
121+
} // namespace

tst/qdomyos-zwift-tests.pro

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ SOURCES += \
5050
Devices/TestFtmsControlPointHandshake.cpp \
5151
Devices/TestServiceSubscriptionPlan.cpp \
5252
Devices/TestRideScenario.cpp \
53+
Devices/TestSimulatedBikeAnnouncement.cpp \
5354
Erg/TestErgTableSelection.cpp \
5455
Erg/TestErgAutoMode.cpp \
5556
main.cpp
@@ -104,6 +105,7 @@ HEADERS += \
104105
Devices/TestFtmsControlPointHandshake.h \
105106
Devices/TestServiceSubscriptionPlan.h \
106107
Devices/TestRideScenario.h \
108+
Devices/TestSimulatedBikeAnnouncement.h \
107109
Erg/ergtabletestsuite.h \
108110
Erg/TestErgTableSelection.h \
109111
Erg/TestErgAutoMode.h \

0 commit comments

Comments
 (0)