chore(locator_by_rssi): c320x240 updates#281
Merged
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe RSSI locator widget is refactored to fix a display-sizing issue by applying LVGL scaling consistently across all UI elements. Signal detection is restructured with explicit type constants for clearer FrSky/ELRS branching, value normalization is isolated by signal type, and runtime safety guards prevent invalid percentage calculations when signal values are unavailable. ChangesSignal Detection & UI Scaling
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdcard/c320x240/SCRIPTS/TOOLS/locator_by_rssi/app.lua (1)
307-320:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the detected signal state for visibility and playback.
The UI visibility and audio gating still rely on
getRSSI(), which only reflects FrSky RSSI. When ELRS is active (signalType =SIGNAL_1RSSorSIGNAL_2RSS),getRSSI()returns 0 even thoughsignalValuecontains a valid signal reading. This leaves the widget stuck on the "No signal found" screen and suppresses beeps/haptics during ELRS operation.Replace visibility checks at lines 307-309 and 319-320, and the audio gate at line 354, to key off
signalTypeandsignalValueinstead.Suggested fix
- visible=function() - return getRSSI() ~= 0 - end + visible=function() + return signalType ~= SIGNAL_NONE + and signalValue ~= nil + and (signalType ~= SIGNAL_RSSI or signalValue > 0) + end @@ - visible=function() - return getRSSI() == 0 - end + visible=function() + return signalType == SIGNAL_NONE + or signalValue == nil + or (signalType == SIGNAL_RSSI and signalValue <= 0) + end @@ - if getRSSI() ~= 0 and getTime() >= nextPlayTime then + if signalType ~= SIGNAL_NONE + and signalValue ~= nil + and (signalType ~= SIGNAL_RSSI or signalValue > 0) + and getTime() >= nextPlayTime then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdcard/c320x240/SCRIPTS/TOOLS/locator_by_rssi/app.lua` around lines 307 - 320, The UI visibility and audio gating currently check getRSSI(), which is zero for ELRS; update the visible functions for the "signal present" and "no signal" boxes and the audio/haptic gating logic to use the ELRS-aware state instead: test signalType against SIGNAL_1RSS or SIGNAL_2RSS and/or check signalValue > 0 (or non-nil) in addition to existing FrSky logic so that widgets and beeps/haptics trigger when signalType == SIGNAL_1RSS or SIGNAL_2RSS and signalValue indicates a valid reading; modify the anonymous visible=function() closures for those boxes and the audio gating condition to evaluate (getRSSI() ~= 0) || ((signalType == SIGNAL_1RSS or signalType == SIGNAL_2RSS) and signalValue > 0) (or equivalent truthy check) and ensure you reference the same signalType/signalValue variables used elsewhere in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdcard/c320x240/SCRIPTS/TOOLS/locator_by_rssi/app.lua`:
- Around line 162-165: The log calls concatenate the raw result of
getValue("RSSI") which may be nil and will throw an error; update the log
expressions to use tostring(v) (e.g., after v = getValue("RSSI")) so nil becomes
"nil" and cannot abort the widget — apply this change to the log calls near
fieldinfo/getValue usage (the log("RSSI: " .. v) instance and the other
concatenations in the same block around lines 187-198), leaving signalValue = v
unchanged so the value is still stored as-is.
- Around line 118-133: The early check that reads txPowerFieldTpwr =
getFieldInfo("TPWR") and returns SIGNAL_NONE when nil blocks ELRS
auto-detection; remove that prerequisite so detection continues even if TPWR is
absent. Specifically, delete or disable the branch that tests txPowerFieldTpwr
== nil and sets signalType = SIGNAL_NONE / return, then allow the subsequent
getFieldInfo("1RSS") and getFieldInfo("2RSS") checks to run and set signalType
to SIGNAL_1RSS or SIGNAL_2RSS as appropriate (leaving TPWR handling in the
value-update phase intact).
---
Outside diff comments:
In `@sdcard/c320x240/SCRIPTS/TOOLS/locator_by_rssi/app.lua`:
- Around line 307-320: The UI visibility and audio gating currently check
getRSSI(), which is zero for ELRS; update the visible functions for the "signal
present" and "no signal" boxes and the audio/haptic gating logic to use the
ELRS-aware state instead: test signalType against SIGNAL_1RSS or SIGNAL_2RSS
and/or check signalValue > 0 (or non-nil) in addition to existing FrSky logic so
that widgets and beeps/haptics trigger when signalType == SIGNAL_1RSS or
SIGNAL_2RSS and signalValue indicates a valid reading; modify the anonymous
visible=function() closures for those boxes and the audio gating condition to
evaluate (getRSSI() ~= 0) || ((signalType == SIGNAL_1RSS or signalType ==
SIGNAL_2RSS) and signalValue > 0) (or equivalent truthy check) and ensure you
reference the same signalType/signalValue variables used elsewhere in the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7dd2ea07-c687-4a3c-967b-2d54125245ed
📒 Files selected for processing (2)
sdcard/c320x240/SCRIPTS/TOOLS/locator_by_rssi/app.luasdcard/c320x240/SCRIPTS/TOOLS/locator_by_rssi/main.lua
Comment on lines
162
to
165
| if fieldinfo then | ||
| local v = getValue("RSSI") | ||
| log("RSSI: " .. v) | ||
| signalType = 2 -- RSSI | ||
| signalValue = v |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the file
find . -name "app.lua" -path "*locator_by_rssi*" | head -20Repository: EdgeTX/edgetx-sdcard
Length of output: 288
🏁 Script executed:
# Check if the file exists and read the relevant lines
if [ -f "sdcard/c320x240/SCRIPTS/TOOLS/locator_by_rssi/app.lua" ]; then
echo "=== File exists, reading lines 160-200 ===";
sed -n '160,200p' "sdcard/c320x240/SCRIPTS/TOOLS/locator_by_rssi/app.lua" | cat -n;
else
echo "File not found at expected path, searching...";
find . -type f -name "app.lua" | grep -i rssi;
fiRepository: EdgeTX/edgetx-sdcard
Length of output: 1214
🏁 Script executed:
# Search for the log function definition in the codebase
rg "function log|log\s*=" --type lua | head -20Repository: EdgeTX/edgetx-sdcard
Length of output: 1575
🏁 Script executed:
# Also check if there are any imports or requires that define log
rg "log" sdcard/c320x240/SCRIPTS/TOOLS/locator_by_rssi/app.lua | head -30Repository: EdgeTX/edgetx-sdcard
Length of output: 686
🏁 Script executed:
# Check the beginning of app.lua to see log function definition
head -50 sdcard/c320x240/SCRIPTS/TOOLS/locator_by_rssi/app.lua | cat -nRepository: EdgeTX/edgetx-sdcard
Length of output: 3136
🏁 Script executed:
# Test Lua nil concatenation behavior to confirm the error
python3 << 'EOF'
# Simulate Lua string concatenation behavior
# In Lua, attempting to concatenate with nil throws: "attempt to concatenate a nil value"
print("Testing Lua string concatenation with nil:")
print("- Lua will throw an error if you try: 'RSSI: ' .. nil")
print("- This occurs BEFORE any function call, so log() never gets called")
print("- The correct fix is to use tostring(v) which converts nil to string 'nil'")
print("\nLua examples:")
print(" 'RSSI: ' .. 5 => 'RSSI: 5' (works)")
print(" 'RSSI: ' .. nil => ERROR: attempt to concatenate a nil value")
print(" 'RSSI: ' .. tostring(nil) => 'RSSI: nil' (works)")
EOFRepository: EdgeTX/edgetx-sdcard
Length of output: 478
🏁 Script executed:
# Search for log function definition in the app.lua file itself
grep -n "function log" sdcard/c320x240/SCRIPTS/TOOLS/locator_by_rssi/app.luaRepository: EdgeTX/edgetx-sdcard
Length of output: 96
🏁 Script executed:
# Get the log function definition
sed -n '68,85p' sdcard/c320x240/SCRIPTS/TOOLS/locator_by_rssi/app.lua | cat -nRepository: EdgeTX/edgetx-sdcard
Length of output: 753
🏁 Script executed:
# Also verify the current state of the problem lines once more with context
sed -n '155,200p' sdcard/c320x240/SCRIPTS/TOOLS/locator_by_rssi/app.lua | cat -nRepository: EdgeTX/edgetx-sdcard
Length of output: 1355
Make the debug logging nil-safe.
getValue() can return nil while telemetry is still coming up. Lua's string concatenation operator (..) throws an error when concatenating with nil, and this error occurs during expression evaluation—before log() is even called. This means Lines 164, 189, and 194 can abort the widget on a missing sample.
The fix is to use tostring(v) which safely converts nil to the string "nil":
Suggested fix
- log("RSSI: " .. v)
+ log("RSSI: %s", tostring(v))
@@
- log("1RSS: " .. v)
+ log("1RSS: %s", tostring(v))
@@
- log("2RSS: " .. v)
+ log("2RSS: %s", tostring(v))Also applies to: 187-198
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdcard/c320x240/SCRIPTS/TOOLS/locator_by_rssi/app.lua` around lines 162 -
165, The log calls concatenate the raw result of getValue("RSSI") which may be
nil and will throw an error; update the log expressions to use tostring(v)
(e.g., after v = getValue("RSSI")) so nil becomes "nil" and cannot abort the
widget — apply this change to the log calls near fieldinfo/getValue usage (the
log("RSSI: " .. v) instance and the other concatenations in the same block
around lines 187-198), leaving signalValue = v unchanged so the value is still
stored as-is.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #279
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores