Skip to content
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.

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

Disable motor stop feature when airmode is enabled #4326

Merged
merged 7 commits into from
Feb 16, 2025

Conversation

haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Feb 2, 2025

This pull request includes changes to the locales/en/messages.json, src/js/Features.js, and src/js/tabs/motors.js files to enhance the functionality and user interface related to the MOTOR_STOP feature. The most important changes include adding a tip message for the MOTOR_STOP feature, updating the feature configuration to include the tip, and modifying the motor initialization logic to handle the MOTOR_STOP feature based on the AIRMODE status.

Enhancements to MOTOR_STOP feature:

  • locales/en/messages.json: Added a new message for the MOTOR_STOP feature tip, indicating that the feature is disabled when AIRMODE is enabled.
  • src/js/Features.js: Updated the MOTOR_STOP feature configuration to include a haveTip attribute, enabling the display of the tip message.
  • src/js/tabs/motors.js: Modified the motor initialization logic to check the status of MOTOR_STOP and AIRMODE, ensuring the MOTOR_STOP feature is only enabled when AIRMODE is disabled.

@haslinghuis haslinghuis added this to the 11.0 milestone Feb 2, 2025
@haslinghuis haslinghuis self-assigned this Feb 2, 2025
Copy link

netlify bot commented Feb 2, 2025

Deploy Preview for origin-betaflight-app ready!

Name Link
🔨 Latest commit 399be77
🔍 Latest deploy log https://app.netlify.com/sites/origin-betaflight-app/deploys/67b12d42cc695000081202ae
😎 Deploy Preview https://deploy-preview-4326.dev.app.betaflight.com
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@haslinghuis haslinghuis changed the title Hide motor stop when airmode is enable Hide motor stop when airmode is enabled Feb 2, 2025
@dmak
Copy link

dmak commented Feb 3, 2025

So you are more inclined to have it hidden. Well, if that is a better option compared to have it disabled, then let it be.

@haslinghuis
Copy link
Member Author

@dmak did not know it was exclusive. So makes sense to hide it in configurator. But still like the info to be in documentation too.

@dmak
Copy link

dmak commented Feb 3, 2025

@dmak did not know it was exclusive. So makes sense to hide it in configurator. But still like the info to be in documentation too.

Well, my idea was that if control is disabled, the user may click on (?) button or navigate to wiki to get more info about why it is disabled. If it is hidden then it rings no bell 😉

@nerdCopter
Copy link
Member

nerdCopter commented Feb 3, 2025

i can predict many i can't click it's 😆 -- probably needs an accompanying help-icon/tooltip.

@haslinghuis haslinghuis changed the title Hide motor stop when airmode is enabled Disable motor stop feature when airmode is enabled Feb 3, 2025
@nerdCopter
Copy link
Member

nerdCopter commented Feb 3, 2025

when you turn it ON in CLI -- maybe need to be gray.
image

hence, hiding it did seem appropriate 😉 🤷‍♂️

@haslinghuis
Copy link
Member Author

haslinghuis commented Feb 3, 2025

Meeeh. cli should also not enable feature when AIRMODE is enabled.

EDIT: When using AIRMODE on a switch think both should be allowed exclusive (MOTOR_STOP for start / end of flight and AIRMODE in flight)

@haslinghuis
Copy link
Member Author

haslinghuis commented Feb 3, 2025

See betaflight/betaflight#14236

EDIT: last change does no longer need changes in firmware.

@nerdCopter
Copy link
Member

nerdCopter commented Feb 10, 2025

rather that toggling it off, why not CSS?

while i did not dig into code, using inspector to find:
original checkbox coloring is #ffbb00

    box-shadow: inset 0 0 0 8px var(--primary-500);
    border-color: var(--primary-500);
    background-color: var(--primary-500);

why not something similar to (but follow other existing disabled css):

checkbox:disabled {
  box-shadow: inset 0 0 0 8px  dimgrey;
  border-color:  dimgrey;
  background-color: dimgrey;
}

edit: seems var --surface-500 or 400 would be a more proper "gray"

@nerdCopter
Copy link
Member

nerdCopter commented Feb 10, 2025

[....] why not CSS?

the .js is rather easy

            $(".escMotorStop").toggle(protocolConfigured);
            $(".escMotorStop :input").attr("disabled", FC.FEATURE_CONFIG.features.isEnabled("AIRMODE"));

maybe :input is wrong, but i could not make it work without.

However, the CSS/Less evades me 1000%. I could not find the correct .switchery in any .less files to apply any .disabled {}

Maybe @VitroidFPV has some clues.

@VitroidFPV
Copy link
Member

I'll have a look at the styles tomorrow, should be doable

@ctzsnooze
Copy link
Member

my 2c... and it's probably like this already, but I can't check:

  • if Airmode is always on then the motor_stop switch should be greyed out, and shown in the 'off' position while greyed out, and the info tip should explain why it is greyed out and off.
  • if Airmode can be enabled via a mode switch, then the user should be able to turn motor_stop on and off.

@nerdCopter
Copy link
Member

my 2c... and it's probably like this already, but I can't check:

* if Airmode is always on then the motor_stop switch should be greyed out, and shown in the 'off' position while greyed out, and the info tip should explain why it is greyed out and off.

* if Airmode can be enabled via a mode switch, then the user should be able to turn motor_stop on and off.

It indeed like so at this moment. However, I proposed that the motor-stop switch still reflect actual CLI value. i.e. can be on, but be grayed-out (and disabled) when airmode is enabled. Such would follow general computer software interfaces. of course, this is "IMHO".

@VitroidFPV
Copy link
Member

Approving, possible to merge if satisfactory

@haslinghuis haslinghuis merged commit 3864cbb into betaflight:master Feb 16, 2025
9 checks passed
@haslinghuis haslinghuis deleted the motor-stop branch February 16, 2025 00:13
@nerdCopter
Copy link
Member

post-merge -- perfect

@haslinghuis
Copy link
Member Author

Yeah CSS is magic. Still cannot grasp @VitroidFPV solution needs the extra switchery class in

image

But guess this is an override. Was trying addClass and removeClass for switchery-disabled and switchery-default in

image

which did not work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

The relation between MOTOR_STOP and AIRMODE should be better documented
5 participants