Zwift shifting by doudar · Pull Request #727 · doudar/SmartSpin2k

doudar · 2026-03-08T16:12:51Z

Implementing ZP

…lity

Move FTMS control point write logic from DirConManager.cpp into BLE_Fitness_Machine_Service::handleDirConWrite(). Move Zwift sync_rx write logic (including auto-subscription UUIDs) from DirConManager.cpp into BLE_Zwift_Service::handleDirConWrite(). DirConManager now calls each service's handleDirConWrite() generically without referencing any service-specific UUIDs or protocol details. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ated methods

… remove handshake state handling

…into Zwift_shifting

…ncy; adjust spacing and update service UUID references

…e command codes for trainer configuration

Copilot

Pull request overview

This PR introduces Zwift virtual shifting support (ZP) by adding a Zwift custom BLE service and an OpenBikeControl BLE service, and extends the DirCon bridge to discover services dynamically and dispatch writes to service-specific handlers.

Changes:

Add Zwift custom BLE service + protocol handling for handshake, riding data, and virtual shift events.
Add OpenBikeControl BLE service (button state + haptic/app-info writes) and advertise it over mDNS.
Refactor DirCon to use a service registration model for discovery, MDNS TXT updates, and write handling callbacks.

Reviewed changes

Copilot reviewed 26 out of 29 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
src/SS2KLog.cpp	Hardens log formatting/sending and adds hex helpers (now templated).
include/SS2KLog.h	Exposes new templated hex helpers.
src/Main.cpp	Forwards physical shifter deltas to Zwift/OpenBikeControl services.
include/Main.h	Adds accessors for last shifter position to prevent shift echo loops.
src/DirConMessage.cpp	Improves unknown-message logging by dumping full message bytes.
src/DirConManager.cpp	Adds service registration + write dispatch, improved subscription tracking, OpenBikeControl mDNS advertising, and notification locking.
include/DirConManager.h	Adds DirCon service registry/write-handler API and subscription entry struct.
src/BLE_Zwift_Service.cpp	New Zwift Ride custom service implementation (handshake, keepalive, gear ratio, shifting).
include/BLE_Zwift_Service.h	Public interface for Zwift service + callback class.
src/BLE_OpenBikeControl_Service.cpp	New OpenBikeControl BLE service implementation (button state notify, haptic/app-info writes).
include/BLE_OpenBikeControl_Service.h	Public interface for OpenBikeControl service + callback classes.
src/BLE_Server.cpp	Wires Zwift/OpenBikeControl services into server startup and update loop; adjusts advertising/scan response.
src/BLE_Heart_Service.cpp	Switches DirCon integration to registerService().
src/BLE_Fitness_Machine_Service.cpp	Switches DirCon integration to registerService() with a write handler for FTMS CP.
src/BLE_Device_Information_Service.cpp	Improves DIS fields (manufacturer/model/hw rev/serial/system id) for interoperability.
src/BLE_Cycling_Speed_Cadence.cpp	Switches DirCon integration to registerService().
src/BLE_Cycling_Power_Service.cpp	Switches DirCon integration to registerService().
src/BLE_Custom_Characteristic.cpp	Minor formatting-only change.
lib/SS2K/include/Constants.h	Adds UUID constants for Zwift and OpenBikeControl services/characteristics.
include/Zwift_Protocol_Messages.h	Adds Zwift protobuf/tag/enum definitions used by the Zwift service.
platformio.ini	Adds a pre-build cleanup script; updates NimBLE-Arduino comment link.
scripts/pre_build_cleanup.py	New pre-build script to remove stale x509 bundle artifacts and force single-job builds.
scripts/pycache/pre_build_cleanup.cpython-313.pyc	Added compiled Python artifact (should not be committed).
sdkconfig.release.old	Updates ESP-IDF config snapshot (tooling/build settings).
sdkconfig.release	Updates ESP-IDF release config (NimBLE/Arduino selective build settings).
include/cert.h	Updates root CA certificate bundle used for GitHub raw fetches.
CHANGELOG.md	Adds a new (currently empty) version section.
Hardware/V3 - Integrated PCB/PCB/SS2K_KiCad_Files/SmartSpin2k-backups/SmartSpin2k-2023-01-21_082918.zip	Hardware backup ZIP (binary).
Hardware/V3 - Integrated PCB/PCB/SS2K_KiCad_Files/SmartSpin2k-backups/SmartSpin2k-2022-10-31_214835.zip	Hardware backup ZIP (binary).

Copilot · 2026-04-15T15:06:07Z

  if (_messageBufferHandle == NULL) {
    ESP_LOGE(LOG_HANDLER_TAG, "Can not send log message. Message Buffer is NULL");
    return;
  }


LogHandler::writev() returns early when _messageBufferHandle is NULL without releasing _logBufferMutex, which can permanently deadlock logging for the rest of the runtime. Ensure the semaphore is always given on all exit paths after a successful xSemaphoreTake (e.g., give the mutex before returning on the NULL handle path).

Copilot · 2026-04-15T15:06:08Z

+  DirConManager::notifyCharacteristic(NimBLEUUID(ZWIFT_RIDE_CUSTOM_SERVICE_UUID), asyncCharacteristic->getUUID(), zData, pos);
+}
+
+bool BLE_Zwift_Service::isConnected() {


BLE_Zwift_Service::isConnected() treats _lastActivityTime == 0 as a recent activity (millis()-0 < timeout), so it will report connected for the first ~10s after boot even with no Zwift client. This can incorrectly route shifter events and keepalives. Consider requiring _lastActivityTime != 0 (and/or tracking actual NimBLE connection state) before returning true.

Suggested change

bool BLE_Zwift_Service::isConnected() {

bool BLE_Zwift_Service::isConnected() {

if (_lastActivityTime == 0) {

return false;

}

Copilot · 2026-04-15T15:06:08Z

+  // Also update lastShifterPosition so FTMSModeShiftModifier doesn't
+  // see this Zwift-driven change as a user shift and echo it back.
+  ss2k->setLastShifterPosition(newShifterPos);
+  SS2K_LOG(getLogTag(), "Gear %d -> shifter position %d", closestIndex + 1, newShifterPos);


applyGearRatio(): closestIndex is already set to i + 1, but the log prints closestIndex + 1, which makes the reported gear off by one. Either keep closestIndex 0-based and add 1 only when presenting, or log closestIndex directly to match the shifter position you apply.

Suggested change

SS2K_LOG(getLogTag(), "Gear %d -> shifter position %d", closestIndex + 1, newShifterPos);

SS2K_LOG(getLogTag(), "Gear %d -> shifter position %d", closestIndex, newShifterPos);

Copilot · 2026-04-15T15:06:08Z

+      char hexBuf[len * 3 + 1];
+      for (size_t i = 0; i < len; i++) {
+        snprintf(hexBuf + i * 3, 4, "%02X ", data[i]);
+      }
+      hexBuf[len * 3] = '\0';
+      SS2K_LOG(DIRCON_LOG_TAG, "Error parsing DirCon message: Unknown identifier %d. Full message (%d bytes): %s", this->Identifier, len, hexBuf);


This uses a variable-length array (char hexBuf[len * 3 + 1];). VLAs are not standard C++ and can fail to compile depending on toolchain flags; they also risk large stack usage when len grows. Prefer a fixed-size buffer based on the protocol max (e.g., DIRCON_RECEIVE_BUFFER_SIZE) or build the hex string incrementally into a std::string.

Suggested change

char hexBuf[len * 3 + 1];

for (size_t i = 0; i < len; i++) {

snprintf(hexBuf + i * 3, 4, "%02X ", data[i]);

}

hexBuf[len * 3] = '\0';

SS2K_LOG(DIRCON_LOG_TAG, "Error parsing DirCon message: Unknown identifier %d. Full message (%d bytes): %s", this->Identifier, len, hexBuf);

char hexBuf[(DIRCON_RECEIVE_BUFFER_SIZE * 3) + 1];

size_t maxBytesToLog = DIRCON_RECEIVE_BUFFER_SIZE;

size_t bytesToLog = len < maxBytesToLog ? len : maxBytesToLog;

for (size_t i = 0; i < bytesToLog; i++) {

snprintf(hexBuf + i * 3, 4, "%02X ", data[i]);

}

hexBuf[bytesToLog * 3] = '\0';

if (bytesToLog < len) {

SS2K_LOG(DIRCON_LOG_TAG, "Error parsing DirCon message: Unknown identifier %d. Full message (%d bytes, truncated to %d bytes): %s", this->Identifier, len, bytesToLog, hexBuf);

} else {

SS2K_LOG(DIRCON_LOG_TAG, "Error parsing DirCon message: Unknown identifier %d. Full message (%d bytes): %s", this->Identifier, len, hexBuf);

}

Copilot · 2026-04-15T15:06:09Z

+
+void BLE_Zwift_Service::sendShiftDown() {
+  SS2K_LOG(getLogTag(), "Sending shift down to Zwift");
+  sendButtonNotification(RideButtonMask::ShiftUpLeft);


sendShiftDown() uses RideButtonMask::ShiftUpLeft, which looks like the wrong button bit for a downshift (there is also ShiftDownLeft / ShiftDownRight). This likely sends the incorrect Zwift button event. Use the appropriate ShiftDown* mask for the downshift path.

Suggested change

sendButtonNotification(RideButtonMask::ShiftUpLeft);

sendButtonNotification(RideButtonMask::ShiftDownLeft);

Copilot · 2026-04-15T15:06:09Z

+static SemaphoreHandle_t s_notifyMutex = xSemaphoreCreateMutex();
+
+void DirConManager::broadcastNotification(const NimBLEUUID& characteristicUuid, uint8_t* data, size_t length, bool onlySubscribers) {


s_notifyMutex is created at static initialization time and is used without a null check. If xSemaphoreCreateMutex() fails (or static init ordering is problematic), later xSemaphoreTake() will dereference a null handle and crash. Create the mutex during DirConManager::start() (or lazily on first use) and guard against nullptr before taking/giving it.

Copilot · 2026-04-15T15:06:09Z

+# Keep build deterministic for generated asm artifacts.
+env.SetOption("num_jobs", 1)


Setting env.SetOption("num_jobs", 1) forces single-threaded builds for all environments, which can significantly increase build times. If the intent is only to avoid stale x509 bundle artifacts, consider limiting the workaround to deleting the stale files (or gating the single-job setting behind an env flag / only enabling it for affected environments).

Suggested change

# Keep build deterministic for generated asm artifacts.

env.SetOption("num_jobs", 1)

# Keep the workaround targeted to the affected generated files so

# normal parallel build performance is preserved.

Copilot · 2026-04-15T15:06:10Z

+void BLE_Zwift_Service::update() {
+  unsigned long now                     = millis();
+  static unsigned long lastadvTime      = 0;
+  static bool advertiseManufacturerData = true;
+  static bool swapAdv                   = false;


BLE_Zwift_Service::update() doesn’t currently gate riding-data notifications behind a real Zwift session/connection, and it doesn’t throttle sends to kZwiftRidingDataIntervalMs (the _lastRidingDataTime member is never used). Add a connection/session check and periodic send logic to avoid spamming BLE/DirCon and wasting CPU.

Copilot · 2026-04-15T15:06:10Z

+    int absDelta = abs(shiftDelta);
+    if (zwiftService.isConnected()) {
+      for (int i = 0; i < absDelta; i++) {
+        if (shiftDelta > 0) {
+          zwiftService.sendShiftUp();


FTMSModeShiftModifier() forwards shifts by calling sendShiftUp/Down in a loop. Those helpers include a vTaskDelay(50ms) per step, so a multi-gear change can stall this task for hundreds of milliseconds (and longer if multiple services are connected). Consider making shift notifications non-blocking (e.g., queue events to a dedicated task/timer) so control-loop timing isn’t impacted.

doudar and others added 21 commits February 28, 2026 22:50

Add Zwift custom BLE service and integrate virtual shifting functiona…

5457dbf

…lity

Controls almost working.

cbf1549

Zwift rxing commands

9ed9e20

eliminated shifting feedback loop

81f0856

in progress dev

60239b2

in progress

16e473d

Enhance DirCon service registration and write handling; remove deprec…

7c3f1ff

…ated methods

Update changelog for version 26.1.31

619115d

Refactor BLE_Zwift_Service to improve session management and logging;…

a67c1a8

… remove handshake state handling

Merge branch 'Zwift_shifting' of https://github.com/doudar/SmartSpin2k …

445304d

…into Zwift_shifting

back to Arduino BLE library

89081f0

Refactor BLE_Zwift_Service for improved code readability and consiste…

dafecdc

…ncy; adjust spacing and update service UUID references

Thread safety and advertisement cleanup

bcec78c

Enhance BLE_Zwift_Service with new payload handling methods and updat…

a1d7e04

…e command codes for trainer configuration

WIP

1607b8b

Fixed some ZP errors

a65ef8c

Update BLE_Server.cpp

fdc0c3b

wip

0e98dc2

templated a couple functions

5cbfcfe

added openBikeControl

063541e

Copilot AI review requested due to automatic review settings April 15, 2026 14:56

Copilot started reviewing on behalf of doudar April 15, 2026 14:56 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Zwift shifting#727

Zwift shifting#727
doudar wants to merge 21 commits intodevelopfrom
Zwift_shifting

doudar commented Mar 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Copilot AI Apr 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	SS2K_LOG(getLogTag(), "Gear %d -> shifter position %d", closestIndex + 1, newShifterPos);
	SS2K_LOG(getLogTag(), "Gear %d -> shifter position %d", closestIndex, newShifterPos);

-      char hexBuf[len * 3 + 1];
-      for (size_t i = 0; i < len; i++) {
-        snprintf(hexBuf + i * 3, 4, "%02X ", data[i]);
-      }
-      hexBuf[len * 3] = '\0';
-      SS2K_LOG(DIRCON_LOG_TAG, "Error parsing DirCon message: Unknown identifier %d. Full message (%d bytes): %s", this->Identifier, len, hexBuf);
+      char hexBuf[(DIRCON_RECEIVE_BUFFER_SIZE * 3) + 1];
+      size_t maxBytesToLog = DIRCON_RECEIVE_BUFFER_SIZE;
+      size_t bytesToLog = len < maxBytesToLog ? len : maxBytesToLog;
+      for (size_t i = 0; i < bytesToLog; i++) {
+        snprintf(hexBuf + i * 3, 4, "%02X ", data[i]);
+      }
+      hexBuf[bytesToLog * 3] = '\0';
+      if (bytesToLog < len) {
+        SS2K_LOG(DIRCON_LOG_TAG, "Error parsing DirCon message: Unknown identifier %d. Full message (%d bytes, truncated to %d bytes): %s", this->Identifier, len, bytesToLog, hexBuf);
+      } else {
+        SS2K_LOG(DIRCON_LOG_TAG, "Error parsing DirCon message: Unknown identifier %d. Full message (%d bytes): %s", this->Identifier, len, hexBuf);
+      }

	sendButtonNotification(RideButtonMask::ShiftUpLeft);
	sendButtonNotification(RideButtonMask::ShiftDownLeft);

		static SemaphoreHandle_t s_notifyMutex = xSemaphoreCreateMutex();

		void DirConManager::broadcastNotification(const NimBLEUUID& characteristicUuid, uint8_t* data, size_t length, bool onlySubscribers) {

		# Keep build deterministic for generated asm artifacts.
		env.SetOption("num_jobs", 1)

Uh oh!

Conversation

doudar commented Mar 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants