add pin config and test logic for the waveshare s3 1.43 inch display#578
add pin config and test logic for the waveshare s3 1.43 inch display#578anhhct wants to merge 1 commit intojniebuhr:masterfrom
Conversation
|
We require contributors to sign our Contributor License Agreement, and we don't have yours on file. In order for us to review and merge your code, please contact @jniebuhr (mdwasp) on Discord to get yourself added. |
📝 WalkthroughWalkthroughThe changes add support for a new 1.43-inch Waveshare AMOLED display variant by introducing a dedicated hardware configuration constant with specific pin mappings and display parameters, and updating the driver's compatibility detection logic to test this new variant alongside the existing 1.75-inch model. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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
🤖 Fix all issues with AI agents
In `@src/display/drivers/AmoledDisplay/pin_config.h`:
- Around line 84-109: The code uses hwConfig.tp_int (-1 for the 1.43" variant)
without guards causing invalid-pin operations; update all touch-wakeup and
touch-init paths to check tp_int >= 0 before using it: wrap calls to
pinMode(hwConfig.tp_int,...), digitalRead(hwConfig.tp_int),
esp_sleep_enable_ext1_wakeup(_BV(hwConfig.tp_int), ...), and any use of
_BV(hwConfig.tp_int) with an if (hwConfig.tp_int >= 0) block, and avoid calling
initTouch()/setPins(hwConfig.tp_rst, hwConfig.tp_int) or call setPins with only
valid pins (guard hwConfig.tp_rst >= 0 and hwConfig.tp_int >= 0) so the
WAKEUP_FROM_TOUCH path and enableTouchWakeup() no longer perform operations on
-1 pins.
| constexpr AmoledHwConfig WAVESHARE_S3_AMOLED_143_HW_CONFIG{ | ||
| .lcd_sdio0 = 11, | ||
| .lcd_sdio1 = 12, | ||
| .lcd_sdio2 = 13, | ||
| .lcd_sdio3 = 14, | ||
| .lcd_sclk = 10, | ||
| .lcd_cs = 9, | ||
| .lcd_rst = 21, | ||
| .lcd_width = 466, | ||
| .lcd_height = 466, | ||
| .lcd_gram_offset_x = 0, | ||
| .lcd_gram_offset_y = 0, | ||
| .lcd_en = 42, | ||
| .i2c_sda = 47, | ||
| .i2c_scl = 48, | ||
| .tp_int = -1, | ||
| .tp_rst = -1, | ||
| .battery_voltage_adc_data = 4, // BAT_ADC is IO4 in the table | ||
| .sd_cs = 38, | ||
| .sd_mosi = 39, | ||
| .sd_miso = 40, | ||
| .sd_sclk = 41, | ||
| .pcf8563_int = 15, // RTC INT is IO15 in the table | ||
| .rotation_175 = 0, | ||
| .mirror_touch = false, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for tp_int usages to understand how it's handled
rg -n 'tp_int' --type=cpp --type=h -C 4
Repository: jniebuhr/gaggimate
Length of output: 7175
🏁 Script executed:
#!/bin/bash
# Check how initTouch is called and if there are guards around tp_int usage
rg -n 'initTouch|WAKEUP_FROM_TOUCH' src/display/drivers/AmoledDisplay/Amoled_DisplayPanel.cpp -B 2 -A 2
Repository: jniebuhr/gaggimate
Length of output: 914
🏁 Script executed:
#!/bin/bash
# Search for TouchDrvFT6X36 and TouchDrvCST92xx setPins implementation
rg -n 'setPins' --type=cpp --type=h -B 2 -A 5
Repository: jniebuhr/gaggimate
Length of output: 8012
🏁 Script executed:
#!/bin/bash
# Find the touch driver header files to see setPins implementation
find . -type f -name "*TouchDrv*.h" | head -20
Repository: jniebuhr/gaggimate
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check the full initTouch function to see if there's any handling for -1 pins
sed -n '223,260p' src/display/drivers/AmoledDisplay/Amoled_DisplayPanel.cpp
Repository: jniebuhr/gaggimate
Length of output: 1321
🏁 Script executed:
#!/bin/bash
# Check if there are any guards or checks for -1 pins elsewhere in the codebase
rg -n 'tp_int.*-1|tp_int\s*==\s*-1|tp_int\s*>=\s*0|tp_int\s*<\s*0' --type=cpp --type=h -C 3
Repository: jniebuhr/gaggimate
Length of output: 589
Critical: tp_int = -1 will cause runtime failures in touch wakeup and sleep paths.
The 1.43" display has no touch interrupt pin, but the sleep/wakeup code unconditionally uses tp_int without checking for -1:
- Line 111:
pinMode(hwConfig.tp_int, INPUT)— fails with invalid pin-1 - Line 114:
digitalRead(hwConfig.tp_int)— fails with invalid pin-1 - Line 121:
esp_sleep_enable_ext1_wakeup(_BV(hwConfig.tp_int), ...)—_BV(-1)causes undefined behavior (negative bit shift)
If enableTouchWakeup() is called and the device enters sleep, the WAKEUP_FROM_TOUCH case will execute these invalid operations and crash.
Additionally, initTouch() unconditionally calls setPins(hwConfig.tp_rst, hwConfig.tp_int) with -1 values, passing the risk downstream to the touch driver library.
Add guard checks (e.g., if (hwConfig.tp_int >= 0)) around all GPIO operations using tp_int in the sleep/wakeup paths, or document that enableTouchWakeup() must not be called for the 1.43" variant.
🤖 Prompt for AI Agents
In `@src/display/drivers/AmoledDisplay/pin_config.h` around lines 84 - 109, The
code uses hwConfig.tp_int (-1 for the 1.43" variant) without guards causing
invalid-pin operations; update all touch-wakeup and touch-init paths to check
tp_int >= 0 before using it: wrap calls to pinMode(hwConfig.tp_int,...),
digitalRead(hwConfig.tp_int), esp_sleep_enable_ext1_wakeup(_BV(hwConfig.tp_int),
...), and any use of _BV(hwConfig.tp_int) with an if (hwConfig.tp_int >= 0)
block, and avoid calling initTouch()/setPins(hwConfig.tp_rst, hwConfig.tp_int)
or call setPins with only valid pins (guard hwConfig.tp_rst >= 0 and
hwConfig.tp_int >= 0) so the WAKEUP_FROM_TOUCH path and enableTouchWakeup() no
longer perform operations on -1 pins.
1 participant
The 1.43 inch waveshare s3 Amoled display (schema) has different pinout compared to the 1.75 inch version.
This add a hardware config and hardware checking to support the 1.43 inch version.
Summary by CodeRabbit