Skip to content

Add keyboard shortcuts to Motor Direction wizard #4641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add keyboard shortcuts to Motor Direction wizard #4641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/components/EscDshotDirection/Body.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ <h4 id="escDshotDirectionDialog-SettingsAutoSaved-Normal" i18n="escDshotDirectio
<div id="escDshotDirectionDialog-WizardDialog" class="display-contents">
<a href="#" id="escDshotDirectionDialog-SpinWizard" class="regular-button" i18n="escDshotDirectionDialog-SpinWizard"></a>
<div id="escDshotDirectionDialog-SpinningWizard" class="display-contents">
<!-- Keyboard shortcuts tooltip -->
<div class="keyboard-shortcuts-tooltip">
<strong>⌨️ Keyboard Shortcuts:</strong>
<span class="shortcut-item"><kbd>Space</kbd> = Spin/Stop Motors</span>
<span class="shortcut-separator">|</span>
<span class="shortcut-item"><kbd>1-8</kbd> = Toggle Direction</span>
</div>

<h4 id="escDshotDirectionDialog-WizardActionHint" i18n="escDshotDirectionDialog-WizardActionHint"></h4>
<h4 id="escDshotDirectionDialog-WizardActionHintSecondLine" i18n="escDshotDirectionDialog-WizardActionHintSecondLine"></h4>
<div id="escDshotDirectionDialog-WizardMotorButtons">
Expand Down
206 changes: 206 additions & 0 deletions src/components/EscDshotDirection/EscDshotDirectionComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,17 @@ class EscDshotDirectionComponent {
this._allMotorsAreSpinning = false;
this._spinDirectionToggleIsActive = true;
this._activationButtonTimeoutId = null;
this._isKeyboardControlEnabled = false;
this._spacebarPressed = false;
this._keyboardEventHandlerBound = false;
this._isWizardActive = false;
this._globalKeyboardActive = false;

// Bind methods to preserve 'this' context - CRITICAL for event handlers
this._handleWizardKeyDown = this._handleWizardKeyDown.bind(this);
this._handleWizardKeyUp = this._handleWizardKeyUp.bind(this);
this._handleGlobalKeyDown = this._handleGlobalKeyDown.bind(this);
this._handleWindowBlur = this._handleWindowBlur.bind(this);

this._contentDiv.load("./components/EscDshotDirection/Body.html", () => {
this._initializeDialog();
Expand Down Expand Up @@ -285,9 +296,196 @@ class EscDshotDirectionComponent {
}
}

_enableGlobalKeyboard() {
if (this._globalKeyboardActive) return;

document.addEventListener("keydown", this._handleGlobalKeyDown, true);
this._globalKeyboardActive = true;
}

_disableGlobalKeyboard() {
document.removeEventListener("keydown", this._handleGlobalKeyDown, true);
this._globalKeyboardActive = false;
}

_handleGlobalKeyDown(event) {
// Only handle spacebar for wizard workflow progression
if (event.code !== "Space" || event.repeat) {
return;
}

// Only process keyboard input if the dialog is actually visible
// Check if either the warning content OR main content is visible
const dialogIsVisible =
(this._domWarningContentBlock && this._domWarningContentBlock.is(":visible")) ||
(this._domMainContentBlock && this._domMainContentBlock.is(":visible"));

if (!dialogIsVisible) {
return;
}

// Step 1: Check the safety checkbox if it's not checked and warning is visible
if (this._domWarningContentBlock.is(":visible") && !this._domAgreeSafetyCheckBox.is(":checked")) {
event.preventDefault();
event.stopPropagation();
this._domAgreeSafetyCheckBox.prop("checked", true);
this._domAgreeSafetyCheckBox.trigger("change");
return;
}

// Step 2: Start wizard if checkbox is checked and wizard isn't open yet
if (this._domWarningContentBlock.is(":visible") && this._domAgreeSafetyCheckBox.is(":checked")) {
event.preventDefault();
event.stopPropagation();
this._onStartWizardButtonClicked();
return;
}

// Step 3: Spin motors if wizard is open but not spinning yet
if (
this._domMainContentBlock.is(":visible") &&
this._domSpinWizardButton.is(":visible") &&
!this._isWizardActive
) {
event.preventDefault();
event.stopPropagation();
// Mark spacebar as pressed since we're transitioning to wizard control while key is down
this._spacebarPressed = true;
this._onSpinWizardButtonClicked();
return;
}

// Step 4: If wizard is active, let the wizard keyboard handler take over
// (no action needed here, the _handleWizardKeyDown will handle it)
}

_enableKeyboardControl() {
if (this._keyboardEventHandlerBound) return;

// CRITICAL: Use capture phase (third parameter = true) for reliable event handling
// This prevents other elements from stopping propagation before we handle the event
document.addEventListener("keydown", this._handleWizardKeyDown, true);
document.addEventListener("keyup", this._handleWizardKeyUp, true);

// SAFETY FEATURE: Stop motors if user switches windows while holding spacebar
window.addEventListener("blur", this._handleWindowBlur);

this._keyboardEventHandlerBound = true;
this._isKeyboardControlEnabled = true;
}

_disableKeyboardControl() {
document.removeEventListener("keydown", this._handleWizardKeyDown, true);
document.removeEventListener("keyup", this._handleWizardKeyUp, true);
window.removeEventListener("blur", this._handleWindowBlur);
this._keyboardEventHandlerBound = false;
this._isKeyboardControlEnabled = false;
this._spacebarPressed = false;
}

_handleWizardKeyDown(event) {
// Only handle events when keyboard control is active
if (!this._isKeyboardControlEnabled || !this._isWizardActive) {
return;
}

// SPACEBAR: Spin all motors (hold to spin, release to stop)
if (event.code === "Space") {
event.preventDefault();
event.stopPropagation();
// CRITICAL: Check !event.repeat to prevent multiple triggers when holding key
if (!this._spacebarPressed && !event.repeat) {
this._spacebarPressed = true;
this._handleSpacebarPress();
}
return;
}

// NUMBER KEYS 1-8: Toggle individual motor direction
if (event.key >= "1" && event.key <= "8" && !event.repeat) {
event.preventDefault();
event.stopPropagation();
const motorIndex = parseInt(event.key) - 1;

if (motorIndex < this._numberOfMotors) {
this._toggleMotorDirection(motorIndex);
}
return;
}
}

_handleWizardKeyUp(event) {
if (!this._isKeyboardControlEnabled || !this._isWizardActive) {
return;
}

// SPACEBAR RELEASE: Stop motors immediately
if (event.code === "Space") {
event.preventDefault();
event.stopPropagation();
if (this._spacebarPressed) {
this._spacebarPressed = false;
this._handleSpacebarRelease();
}
}
}

_handleSpacebarPress() {
this._motorDriver.spinAllMotors();
}

_handleSpacebarRelease() {
this._motorDriver.stopAllMotorsNow();
}

_handleWindowBlur() {
// SAFETY FEATURE: Stop motors if user switches windows while holding spacebar
if (this._spacebarPressed) {
this._spacebarPressed = false;
this._handleSpacebarRelease();
}
}

_toggleMotorDirection(motorIndex) {
const button = this._wizardMotorButtons[motorIndex];
const currentlyReversed = button.hasClass(EscDshotDirectionComponent.PUSHED_BUTTON_CLASS);

if (currentlyReversed) {
button.removeClass(EscDshotDirectionComponent.PUSHED_BUTTON_CLASS);
this._motorDriver.setEscSpinDirection(motorIndex, DshotCommand.dshotCommands_e.DSHOT_CMD_SPIN_DIRECTION_1);
} else {
button.addClass(EscDshotDirectionComponent.PUSHED_BUTTON_CLASS);
this._motorDriver.setEscSpinDirection(motorIndex, DshotCommand.dshotCommands_e.DSHOT_CMD_SPIN_DIRECTION_2);
}
}

open() {
// Enable global keyboard when dialog is opened
this._enableGlobalKeyboard();
}

close() {
// Disable keyboard handlers first to prevent any new input
this._disableKeyboardControl();
this._disableGlobalKeyboard();

// If wizard is active, deactivate buttons but DON'T clear the flag yet
// This ensures pending motor direction commands complete
if (this._isWizardActive) {
this._deactivateWizardMotorButtons();
}

// Stop motors (this adds stop commands to the queue)
this._motorDriver.stopAllMotorsNow();

// Deactivate motor driver - this tells queue to stop AFTER processing current commands
// This is critical - it allows direction change + save commands to complete
this._motorDriver.deactivate();

// Clear wizard flag after motor driver deactivation
this._isWizardActive = false;

// Reset GUI last
this._resetGui();
}
Comment on lines 467 to 490
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify cleanup sequence ensures motor commands complete.

The cleanup sequence is carefully ordered:

  1. Disable keyboard handlers (lines 469-470)
  2. Deactivate wizard buttons (line 475)
  3. Stop motors (line 479)
  4. Deactivate motor driver (line 483)
  5. Clear wizard flag (line 486)
  6. Reset GUI (line 489)

This ordering allows pending direction change commands to complete before shutdown. However, the effectiveness depends on _motorDriver.deactivate() and stopAllMotorsNow() implementation details.

Verify that the motor driver's command queue processes all pending direction changes before fully stopping:

🏁 Script executed:

#!/bin/bash
# Check motor driver implementation for queue processing during deactivation

# Find the motor driver file
motor_driver_file=$(fd -t f "EscDshotDirectionMotorDriver" --extension js)

if [ -n "$motor_driver_file" ]; then
    echo "=== Motor Driver deactivate() method ==="
    ast-grep --pattern $'deactivate() {
        $$$
    }'
    
    echo ""
    echo "=== Motor Driver stopAllMotorsNow() method ==="
    ast-grep --pattern $'stopAllMotorsNow() {
        $$$
    }'
else
    echo "Motor driver file not found"
fi

Length of output: 88

🏁 Script executed:

#!/bin/bash
# Locate motor driver file(s)
echo "=== Files matching 'MotorDriver' ==="
fd -i MotorDriver -e js

# Inspect implementations of stopAllMotorsNow() and deactivate()
echo -e "\n=== stopAllMotorsNow() definitions ==="
rg -n --type js 'stopAllMotorsNow\s*\(' -C3

echo -e "\n=== deactivate() definitions ==="
rg -n --type js 'deactivate\s*\(' -C3

echo -e "\n=== Class definitions for MotorDriver ==="
rg -n --type js 'class\s+.*MotorDriver' -C3

Length of output: 5202

🏁 Script executed:

#!/bin/bash
# Show the first 200 lines of the motor driver file to locate queue initialization
sed -n '1,200p' src/components/EscDshotDirection/EscDshotDirectionMotorDriver.js

Length of output: 4560

Critical: stopAllMotorsNow() drops pending commands
In EscDshotDirectionMotorDriver.js (lines 48–51), stopAllMotorsNow() calls this._EscDshotCommandQueue.clear(), which discards any queued direction-change commands before issuing the stop. If you need to let those commands complete, remove or defer the clear() call (or replace it with stopWhenEmpty()) so pending operations aren’t lost.

🤖 Prompt for AI Agents 
In src/components/EscDshotDirection/EscDshotDirectionComponent.js lines 467-490:
the current sequence calls this._motorDriver.stopAllMotorsNow(), but
stopAllMotorsNow() in EscDshotDirectionMotorDriver.js clears the command queue
and drops pending direction-change commands; change the integration so we do NOT
call a method that clears the queue while we still need queued commands to
complete. Fix by either (A) removing the clear() call inside stopAllMotorsNow()
and instead implement a stopWhenEmpty() behavior in the motor driver that waits
for the queue to drain then issues stops, or (B) add an optional parameter to
stopAllMotorsNow({force:false}) and call stopWhenEmpty() /
stopAllMotorsNow({force:false}) here. Ensure the motorDriver.deactivate()
behavior still prevents new commands but allows queued commands to finish, and
update tests/comments accordingly.


Expand Down Expand Up @@ -363,13 +561,21 @@ class EscDshotDirectionComponent {
this._motorDriver.spinAllMotors();

this._activateWizardMotorButtons(0);

// NEW: Enable keyboard shortcuts when wizard starts spinning
this._isWizardActive = true;
this._enableKeyboardControl();
}

_onStopWizardButtonClicked() {
this._domSpinWizardButton.toggle(true);
this._domSpinningWizard.toggle(false);
this._motorDriver.stopAllMotorsNow();
this._deactivateWizardMotorButtons();

// NEW: Disable keyboard shortcuts when wizard stops
this._disableKeyboardControl();
this._isWizardActive = false;
}

_toggleMainContent(value) {
Expand Down
44 changes: 44 additions & 0 deletions src/css/tabs/motors.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,50 @@
#escDshotDirectionDialog-Content {
flex-grow: 1;
}

// Keyboard shortcuts tooltip
.keyboard-shortcuts-tooltip {
background-color: var(--surface-200);
border-left: 3px solid var(--accent-color);
border-radius: 4px;
padding: 10px 15px;
margin: 10px 0;
text-align: center;
font-size: 0.9em;
display: flex;
align-items: center;
justify-content: center;
flex-wrap: wrap;
gap: 8px;

strong {
color: var(--accent-text);
margin-right: 8px;
}

.shortcut-item {
display: inline-flex;
align-items: center;
gap: 4px;
}

kbd {
background-color: var(--surface-300);
border: 1px solid var(--surface-500);
border-radius: 3px;
padding: 2px 6px;
font-family: monospace;
font-size: 0.85em;
font-weight: bold;
box-shadow: 0 1px 2px rgba(0, 0, 0, 0.1);
}

.shortcut-separator {
color: var(--surface-500);
margin: 0 4px;
}
}

#dialog-mixer-reset {
width: 400px;
height: fit-content;
Expand Down
1 change: 1 addition & 0 deletions src/js/tabs/motors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1353,6 +1353,7 @@ motors.initialize = async function (callback) {

$("#escDshotDirectionDialog-Open").click(function () {
$(document).on("keydown", onDocumentKeyPress);
escDshotDirectionComponent.open();
domEscDshotDirectionDialog[0].showModal();
});

Expand Down