Disable UDev Rule for Radxa Display 10 FHD#9503
Disable UDev Rule for Radxa Display 10 FHD#9503FlorianKohn wants to merge 3 commits intoarmbian:mainfrom
Conversation
github-actions
bot
added
05
📝 WalkthroughWalkthroughA udev rule for the Goodix touchscreen in the Rock 5B Plus board configuration was modified to include a PROGRAM check condition. The rule now verifies a specific display configuration exists in the boot environment before applying the touchscreen calibration matrix assignment. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 the current code and only fix it if needed.
Inline comments:
In `@config/boards/rock-5b-plus.conf`:
- Line 28: The udev rule written into 99-radxa-display8hd-touch.rules currently
uses a bare grep against /boot/armbianEnv.txt which will match commented or
partial strings; change the PROGRAM check to only return true when the active
overlay assignment explicitly contains the exact token
rock-5b-plus-radxa-display-8hd (uncommented and as a full token), e.g. by
matching the overlays= (or overlay=) assignment line and verifying the token is
present as a comma/space-delimited item; update the echo that creates the rule
so PROGRAM== uses that stricter test against /boot/armbianEnv.txt.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1efb14df-170b-4dc1-8172-62729452b20f
📒 Files selected for processing (1)
config/boards/rock-5b-plus.conf
There was a problem hiding this comment.
🧹 Nitpick comments (1)
config/boards/rock-5b-plus.conf (1)
28-28: Good improvement: Pattern now correctly excludes commented overlay entries.The
^[[:space:]]*overlays=anchor ensures that lines starting with#(comments) won't match, addressing the main concern from previous feedback.For stricter full-token matching (preventing a hypothetical
rock-5b-plus-radxa-display-8hd-extendedfrom falsely matching), you could use extended regex with word boundaries:-PROGRAM=="/bin/grep -q '^[[:space:]]*overlays=.*rock-5b-plus-radxa-display-8hd' /boot/armbianEnv.txt" +PROGRAM=="/bin/grep -Eq '^[[:space:]]*overlays=.*(^|[[:space:]])rock-5b-plus-radxa-display-8hd([[:space:]]|$)' /boot/armbianEnv.txt"However, given Armbian's overlay naming conventions, this edge case is unlikely to occur in practice—the current implementation should work correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/boards/rock-5b-plus.conf` at line 28, The current grep pattern in the echo that writes to 99-radxa-display8hd-touch.rules (the PROGRAM=="/bin/grep -q '^[[:space:]]*overlays=.*rock-5b-plus-radxa-display-8hd' /boot/armbianEnv.txt" fragment) successfully avoids commented lines but could still false-positive match longer tokens like rock-5b-plus-radxa-display-8hd-extended; tighten the pattern used in PROGRAM== (or switch to an extended-regex grep invocation) so the overlay name is matched as a full token (e.g., require end-of-token such as end-of-line, comma, or whitespace after the exact overlay) while preserving the existing ^[[:space:]]*overlays= anchor and leave the ENV{LIBINPUT_CALIBRATION_MATRIX} assignment unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@config/boards/rock-5b-plus.conf`:
- Line 28: The current grep pattern in the echo that writes to
99-radxa-display8hd-touch.rules (the PROGRAM=="/bin/grep -q
'^[[:space:]]*overlays=.*rock-5b-plus-radxa-display-8hd' /boot/armbianEnv.txt"
fragment) successfully avoids commented lines but could still false-positive
match longer tokens like rock-5b-plus-radxa-display-8hd-extended; tighten the
pattern used in PROGRAM== (or switch to an extended-regex grep invocation) so
the overlay name is matched as a full token (e.g., require end-of-token such as
end-of-line, comma, or whitespace after the exact overlay) while preserving the
existing ^[[:space:]]*overlays= anchor and leave the
ENV{LIBINPUT_CALIBRATION_MATRIX} assignment unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2d735cbb-74cf-497d-af61-31c7419f525f
📒 Files selected for processing (1)
config/boards/rock-5b-plus.conf
1 participant
Description
The change is needed to ensure compatibility with the Radxa-Display-10-FHD, which does not require a calibration matrix.
As of now, this udev rule also applies to the 10-inch display, since it only matched the
goodix-tsdriver, which is the same for both the Radxa-Display-8-HD and the Radxa-Display-10-FHD.Documentation summary for the change
The rule now includes a
PROGRAMdirective that uses grep to only apply this rule when theRadxa-Display-8-HDoverlay is loaded.How Has This Been Tested?
I tested this fix on my Radxa 5B Plus with the Radxa Display 10 FHD and verified that the udev rule no longer applies.
However, as I do not own the Radxa Display 8 HD, I am unable to test whether it still works as before. Maybe @HeyMeco can help with this, as they originally added this rule in Pull Request 8972.
Checklist:
Summary by CodeRabbit