-
-
Notifications
You must be signed in to change notification settings - Fork 999
Removed wording "Roll" in Pitch sliders #4605
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughUpdated three English localization entries in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
locales/en/messages.json (1)
4538-4539
: Standardize line break tag.Use
<br />
(self-closing) to match the rest of the file and avoid mixed markup.Apply this diff:
- "message": "Pitch Tracking:<br><i><small>Pitch: P, I & FF</small></i>", + "message": "Pitch Tracking:<br /><i><small>Pitch: P, I & FF</small></i>",
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
locales/en/messages.json
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-26T11:37:09.758Z
Learnt from: haslinghuis
PR: betaflight/betaflight-configurator#4490
File: locales/en/messages.json:1534-1545
Timestamp: 2025-05-26T11:37:09.758Z
Learning: In the Betaflight Configurator repository, only English messages should be added directly to `locales/en/messages.json`; other language translations are handled via Crowdin automation and should not be manually updated in PRs.
Applied to files:
locales/en/messages.json
🔇 Additional comments (1)
locales/en/messages.json (1)
4530-4531
: ```shell
#!/bin/bash
set -euo pipefailecho '== rg --debug: pattern Pitch[:\s-]*Roll =='
rg --debug -nI --hidden -S -P -C3 'Pitch[:\s-]*Roll' || trueecho '== rg (no debug): Pitch[:\s-]*Roll =='
rg -nI --hidden -S -P -C3 'Pitch[:\s-]*Roll' || trueecho '== rg variants (Pitch.*Roll, exact etc.) =='
rg -nI --hidden -S -P -C3 'Pitch.*Roll' || true
rg -nI --hidden -S -P -C3 'Pitch:Roll' || true
rg -nI --hidden -S -P -C3 'Pitch-Roll' || true
rg -nI --hidden -S -P -C3 'Pitch Roll' || trueecho '== search for "Pitch" occurrences (context) =='
rg -nI --hidden -S -C3 'Pitch' || trueecho '== find messages.json files and search within them =='
find . -type f -name 'messages.json' -print0 | xargs -0 -r rg -n --hidden -S -P -C3 'Pitch[:\s-]*Roll' || trueecho '== show locales/en/messages.json around lines 4520-4540 =='
if [ -f locales/en/messages.json ]; then
sed -n '4520,4540p' locales/en/messages.json || true
else
echo 'locales/en/messages.json not found'
fiecho '== done =='
</blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
This is intentional to signify that it is a multiplier over the roll values. |
Perhaps we keep the grammatical fixes |
|
Preview URL: https://37b14b31.betaflight-configurator.pages.dev |
@haslinghuis @CarviFPV , I checked your changes (in english version), this seems to be good, thanks! |
@Canaill51 Keeping Roll here is intentional to signify it is a multiplier over the roll values. |
@limonspb - what do you think ? |
i believe this is true. |
@CarviFPV do you care to update the tooltip instead to point out the ratio here ? |
Yes, but I’m not sure how to rephrase it. Do you have an idea? |
@nerdCopter is good in phrasing |
@coderabbitai, i'm lazy today. propose an update to ONLY the tooltip in regards to this PR conversation (only the english message.json). The idea is to explain to the general audience via the tooltip that the Roll:Pitch is a ratio. |
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed:
Length of output: 913 @nerdCopter, here's my proposed update to the tooltip text to clarify the ratio concept: "pidTuningPidSlidersHelp": {
"message": "Sliders to adjust the aircraft flight characteristics (PID gains)<br /><br />Damping (D gain): Resists fast movement, minimises P oscillation.<br /><br />Tracking (P and I gain): Enhances the responsiveness of the aircraft; if too high, may cause trilling or oscillation.<br /><br />Stick Response (Feedforward): Increases the responsiveness of the aircraft to faster stick movements.<br /><br />Drift - Wobble (I gain, expert): Fine adjustment of I.<br /><br />Dynamic D (D Max, expert): Sets the maximum amount that D can be boosted to during fast movements.<br /><br />Pitch Damping (Pitch:Roll D ratio, expert): Adjusts the ratio of pitch damping relative to roll damping. Increases the amount of damping on pitch relative to roll.<br /><br />Pitch Tracking (Pitch:Roll P, I and F ratio, expert): Adjusts the ratio of pitch stabilisation relative to roll stabilisation. Increases stabilising strength on pitch relative to roll.<br /><br />Master Multiplier (all gains, expert): Raises or lowers all the PID gains, keeping their proportions constant."
} Key changes:
This makes it crystal clear that these sliders control ratios (pitch-to-roll) while keeping the existing helpful descriptions about what they do. |
Removed the word "Roll" from the Pitch Sliders wich had confusing some Users. Issue: #4574
Summary by CodeRabbit