feat: transport driver orientation from config.yaml - default 0 rotation. - #355
feat: transport driver orientation from config.yaml - default 0 rotation.#355caveman99 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughFramebuffer initialization now reads ChangesFramebuffer rotation
Estimated code review effort: 1 (Trivial) | ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@source/graphics/driver/FBDriver.cpp`:
- Around line 39-44: Update the MESHTASTIC_FB_ROTATION parsing in the
framebuffer rotation initialization to accept only values 0 through 3, removing
the rot & 3 masking behavior. Map each valid value explicitly to its
corresponding lv_display_rotation_t constant, while malformed or out-of-range
values fall back to 0° or emit a warning.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6709c46e-417f-43d7-85b9-df760a4afd17
📒 Files selected for processing (1)
source/graphics/driver/FBDriver.cpp
| /* Config-driven rotation: (0/1/2/3 = 0/90/180/270) | ||
| * from the YAML (Display.Rotate + OffsetRotate)*/ | ||
| const char *fbrot = getenv("MESHTASTIC_FB_ROTATION"); | ||
| int rot = fbrot ? atoi(fbrot) : 0; | ||
| if (rot) | ||
| lv_display_set_rotation(display, (lv_display_rotation_t)(rot & 3)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate FBDriver.cpp and relevant references =="
git ls-files | grep -F 'source/graphics/driver/FBDriver.cpp' || true
rg -n "MESHTASTIC_FB_ROTATION|lv_display_set_rotation|Display\.Rotate|OffsetRotate" .
echo
echo "== relevant source section =="
if [ -f source/graphics/driver/FBDriver.cpp ]; then
nl -ba source/graphics/driver/FBDriver.cpp | sed -n '1,120p'
fi
echo
echo "== deterministic atoi/rotation behavior probe =="
python3 - <<'PY'
def current(mask):
vals = ["", None, "0", "1", "2", "3", "4", "5", "-1", "abc"]
for v in vals:
import ctypes
# ctypes.c_int.from_buffer_copy mirrors standard atoi-ish semantics for these literal examples?
# Use Python int cast for signed textual values; invalid ASCII values are not covered by atoi probe.
if v is None:
rot = 3
else:
intlike = int(v) if (v.lstrip("-").isdigit() or (v.startswith("-") and v[1:].isdigit())) else 0
rot = intlike & mask
print(f"value={v!r:7} rot={rot}")
print("rotation_t enum likely 0..3")
current(3)
PYRepository: meshtastic/device-ui
Length of output: 742
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate FBDriver.cpp and relevant references =="
git ls-files | grep -F 'source/graphics/driver/FBDriver.cpp' || true
rg -n "MESHTASTIC_FB_ROTATION|lv_display_set_rotation|Display\.Rotate|OffsetRotate" .
echo
echo "== relevant source section =="
if [ -f source/graphics/driver/FBDriver.cpp ]; then
nl -ba source/graphics/driver/FBDriver.cpp | sed -n '1,120p'
fi
echo
echo "== deterministic atoi/rotation behavior probe =="
python3 - <<'PY'
def current(mask):
vals = ["", None, "0", "1", "2", "3", "4", "5", "-1", "abc"]
for v in vals:
if v is None:
rot = 3
else:
intlike = int(v) if (v.lstrip("-").isdigit() or (v.startswith("-") and v[1:].isdigit())) else 0
rot = intlike & mask
print(f"value={v!r:7} rot={rot}")
print("rotation_t enum likely 0..3")
current(3)
PYRepository: meshtastic/device-ui
Length of output: 742
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== read relevant source section =="
awk 'NR>=1 && NR<=120 { printf("%5d\t%s\n", NR, $0) }' source/graphics/driver/FBDriver.cpp
echo
echo "== LVGL rotation enum/header references in repo =="
rg -n "typedef enum|enum \\{|LV_DISPLAY_ROTATION|lv_display_rotation_t|lv_display_set_rotation" -S . || true
echo
echo "== deterministic mask behavior probe =="
python3 - <<'PY'
def mask_val(s):
rot = int(s or "0") & 3
return rot
for s in ["", "0", "1", "2", "3", "4", "5", "99", "-1", "-2", "-3"]:
try:
print(f"value={s!r:>6} masked_rotation={mask_val(s)}")
except Exception as e:
print(f"value={s!r:>6} parsed_error={e}")
PYRepository: meshtastic/device-ui
Length of output: 4423
🌐 Web query:
LVGL lv_display_rotation_t LV_DISPLAY_ROTATION_0 90 180 270 lv_display_set_rotation
💡 Result:
In LVGL, the lv_display_rotation_t enumeration defines the available rotation angles for a display, supporting 90-degree increments [1][2]. The available values are: LV_DISPLAY_ROTATION_0 LV_DISPLAY_ROTATION_90 LV_DISPLAY_ROTATION_180 LV_DISPLAY_ROTATION_270 The function lv_display_set_rotation(disp, rotation) is used to configure the display orientation [2][3]. When this function is called, LVGL internally swaps the horizontal and vertical resolutions of the display [1][2]. Important Considerations: 1. Internal Resolution Swap Only: lv_display_set_rotation updates LVGL's internal understanding of the display dimensions but does not automatically perform the actual pixel rotation on your hardware [1][4]. 2. Hardware/Software Requirements: If your display driver or hardware does not natively support rotation, you must handle the pixel transformation yourself [1][4]. This is typically done within the display's Flush Callback function using the lv_draw_sw_rotate function to rotate the buffer [1][5]. 3. Events: Changing the rotation triggers an event (e.g., LV_EVENT_SIZE_CHANGED or LV_EVENT_RESOLUTION_CHANGED, depending on the version) to allow the application to perform necessary hardware reconfigurations [1][4][2]. 4. Area Rotation: The function lv_display_rotate_area(disp, &area) is available to rotate specific rendered areas according to the currently set display rotation [1][4][2].
Citations:
- 1: https://docs.lvgl.io/latest/en/html/main-modules/display/rotation.html
- 2: https://lvgl.io/docs/open/9.5/API/display/lv_display_h.html
- 3: https://lvgl.io/docs/open/api/display/lv_display_h
- 4: https://lvgl.io/docs/open/main-modules/display/rotation
- 5: https://lvgl.io/docs/open/9.5/main-modules/display/rotation.html
🌐 Web query:
LVGL lv_display_rotation_t underlying enum values 0 1 2 3
💡 Result:
In LVGL, the lv_display_rotation_t enum defines the orientation of a display [1][2]. The underlying integer values associated with these constants are as follows: LV_DISPLAY_ROTATION_0 = 0 LV_DISPLAY_ROTATION_90 = 1 LV_DISPLAY_ROTATION_180 = 2 LV_DISPLAY_ROTATION_270 = 3 These constants are used with functions such as lv_display_set_rotation to specify the desired display orientation [1][3]. Setting a rotation will swap the horizontal and vertical resolutions internally within LVGL [3][4].
Citations:
- 1: https://lvgl.io/docs/open/api/display/lv_display_h
- 2: https://lvgl.io/docs/open/9.5/API/display/lv_display_h
- 3: https://lvgl.io/docs/open/main-modules/display/rotation
- 4: https://lvgl.io/docs/open/9.3/API/display/lv_display
Reject invalid MESHTASTIC_FB_ROTATION values instead of masking them.
rot & 3 silently maps unsupported values to valid LVGL rotations: 4 becomes 0°, 5 becomes 90°, and -1 becomes 270°. Validate the parsed value in the advertised 0..3 range and map the four constants explicitly; malformed values should fall back to 0 or emit a warning.
🤖 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 `@source/graphics/driver/FBDriver.cpp` around lines 39 - 44, Update the
MESHTASTIC_FB_ROTATION parsing in the framebuffer rotation initialization to
accept only values 0 through 3, removing the rot & 3 masking behavior. Map each
valid value explicitly to its corresponding lv_display_rotation_t constant,
while malformed or out-of-range values fall back to 0° or emit a warning.
Source: MCP tools
caveman99
changed the title
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. An unexpected error occurred while generating fixes: Repository rule violations found Required status check "Trunk Check Runner" is expected. |
1 participant
This is needed for meshtasticd to specify framebuffer orientation from the confg.yaml.
Summary by CodeRabbit
MESHTASTIC_FB_ROTATIONenvironment variable.