Skip to content

New kernel drivers and device migrations#563

Open
KenVanHoeylandt wants to merge 14 commits into
mainfrom
develop
Open

New kernel drivers and device migrations#563
KenVanHoeylandt wants to merge 14 commits into
mainfrom
develop

Conversation

@KenVanHoeylandt

@KenVanHoeylandt KenVanHoeylandt commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features
    • Added touch calibration workflow with a “Calibrate” action in display settings, plus setup options to mark calibration as supported vs required.
    • Updated hardware bindings so supported board profiles use concrete display/touch/backlight configurations (including ILI9341/ST7789 with XPT2046 touch and LEDC backlight where applicable).
    • Added configurable gamma-curve selection for supported panels.
  • Bug Fixes
    • Improved touchscreen coordinate/calibration behavior and strengthened LVGL lock handling in the timezone UI.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The pull request migrates device profiles from legacy display and touch implementations to devicetree-backed modules. It adds ILI9341, ST7789 i8080, HX8357, XPT2046 soft-SPI, ESP32 i8080, and GPIO hog drivers. Touch calibration becomes configurable through device properties and Kconfig, with new LVGL APIs, setup integration, calibration UI, and runtime persistence. Device property generation is refactored to use flattened keys, and devicetree integer range validation is added.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.28% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main change: new kernel drivers were added and multiple devices were migrated to them.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch develop

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (4)
Devices/cyd-2432s028rv3/source/module.cpp (1)

8-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider using the new gpio-hog binding instead of a bespoke module for LED pin setup.

This PR adds a tactility,gpio-hog driver for exactly this kind of "set GPIO to fixed level at boot" use case. Migrating the RGB LED off/on logic to a gpio-hog devicetree node would keep this board consistent with the devicetree-driven pattern the rest of the PR establishes, and let this file (and its module.cpp) be removed entirely.

Drivers/xpt2046-softspi-module/README.md (1)

7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use markdown link syntax for the datasheet URL.

A bare URL works but is fragile in some renderers. Consider wrapping it in proper link syntax.

🔗 Proposed fix
-See https://grobotronics.com/images/datasheets/xpt2046-datasheet.pdf
+See [XPT2046 Datasheet](https://grobotronics.com/images/datasheets/xpt2046-datasheet.pdf)
Drivers/xpt2046-softspi-module/include/drivers/xpt2046_softspi.h (1)

18-19: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value

Consider uint32_t for x_max/y_max to match devicetree int type.

The binding YAML defines x-max and y-max as type: int (typically 32-bit in devicetree), but the config struct uses uint16_t. Values above 65535 would silently truncate. While touch panel resolutions are well within uint16_t range, using uint32_t would avoid any potential mismatch with the devicetree int type and prevent future surprises.

♻️ Proposed refactor
-    uint16_t x_max;
-    uint16_t y_max;
+    uint32_t x_max;
+    uint32_t y_max;
Tactility/Source/lvgl/Lvgl.cpp (1)

1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate "lock → get default indev → build/clear calibration → apply" skeleton across 4 sites. The same pattern of building an LvglPointerCalibration from a TouchCalibrationSettings (or clearing it) and applying it to lvgl_pointer_get_default() under the LVGL lock is repeated verbatim in Lvgl.cpp and three places in TouchCalibration.cpp. A single helper (e.g. applyTouchCalibration(const TouchCalibrationSettings*) in the touch settings module) would remove this duplication and keep the lock/lookup/apply logic in one place.

  • Tactility/Source/lvgl/Lvgl.cpp#L76-93: extract the load-settings-and-apply-to-default-indev block into a shared helper called from here.
  • Tactility/Source/app/touchcalibration/TouchCalibration.cpp#L131-144: replace the inline calibration construction + lock/apply with a call to the same shared helper, passing the freshly computed settings.
  • Tactility/Source/app/touchcalibration/TouchCalibration.cpp#L181-190: replace with a call to the helper's "clear" path (or pass nullptr/no-settings to the shared helper).
  • Tactility/Source/app/touchcalibration/TouchCalibration.cpp#L192-213: replace the load-from-disk-and-apply block with the same shared helper used in Lvgl.cpp.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9663ee99-0112-4b1e-83a6-15f77698637c

📥 Commits

Reviewing files that changed from the base of the PR and between fa4a6e2 and f311038.

📒 Files selected for processing (100)
  • Devices/cyd-2432s024r/CMakeLists.txt
  • Devices/cyd-2432s024r/Source/Configuration.cpp
  • Devices/cyd-2432s024r/Source/devices/Display.cpp
  • Devices/cyd-2432s024r/Source/devices/Display.h
  • Devices/cyd-2432s024r/cyd,2432s024r.dts
  • Devices/cyd-2432s024r/device.properties
  • Devices/cyd-2432s024r/devicetree.yaml
  • Devices/cyd-2432s024r/source/module.cpp
  • Devices/cyd-2432s028r/CMakeLists.txt
  • Devices/cyd-2432s028r/Source/Configuration.cpp
  • Devices/cyd-2432s028r/Source/devices/Display.cpp
  • Devices/cyd-2432s028r/Source/devices/Display.h
  • Devices/cyd-2432s028r/Source/module.cpp
  • Devices/cyd-2432s028r/cyd,2432s028r.dts
  • Devices/cyd-2432s028r/device.properties
  • Devices/cyd-2432s028r/devicetree.yaml
  • Devices/cyd-2432s028r/source/module.cpp
  • Devices/cyd-2432s028rv3/CMakeLists.txt
  • Devices/cyd-2432s028rv3/Source/Configuration.cpp
  • Devices/cyd-2432s028rv3/Source/devices/Display.cpp
  • Devices/cyd-2432s028rv3/Source/devices/Display.h
  • Devices/cyd-2432s028rv3/Source/module.cpp
  • Devices/cyd-2432s028rv3/cyd,2432s028rv3.dts
  • Devices/cyd-2432s028rv3/device.properties
  • Devices/cyd-2432s028rv3/devicetree.yaml
  • Devices/cyd-2432s028rv3/source/module.cpp
  • Devices/cyd-e32r28t/CMakeLists.txt
  • Devices/cyd-e32r28t/Source/Configuration.cpp
  • Devices/cyd-e32r28t/Source/devices/Display.cpp
  • Devices/cyd-e32r28t/Source/devices/Display.h
  • Devices/cyd-e32r28t/cyd,e32r28t.dts
  • Devices/cyd-e32r28t/device.properties
  • Devices/cyd-e32r28t/devicetree.yaml
  • Devices/cyd-e32r28t/source/module.cpp
  • Devices/cyd-e32r32p/device.properties
  • Devices/elecrow-crowpanel-basic-28/device.properties
  • Devices/elecrow-crowpanel-basic-35/device.properties
  • Devices/lilygo-thmi/CMakeLists.txt
  • Devices/lilygo-thmi/Source/Configuration.cpp
  • Devices/lilygo-thmi/Source/Init.cpp
  • Devices/lilygo-thmi/Source/devices/Display.cpp
  • Devices/lilygo-thmi/Source/devices/Display.h
  • Devices/lilygo-thmi/Source/devices/Power.cpp
  • Devices/lilygo-thmi/Source/devices/Power.h
  • Devices/lilygo-thmi/device.properties
  • Devices/lilygo-thmi/devicetree.yaml
  • Devices/lilygo-thmi/lilygo,thmi.dts
  • Devices/lilygo-thmi/source/module.cpp
  • Devices/unphone/device.properties
  • Drivers/XPT2046/Source/Xpt2046Touch.cpp
  • Drivers/XPT2046/Source/Xpt2046Touch.h
  • Drivers/XPT2046SoftSPI/Source/Xpt2046SoftSpi.cpp
  • Drivers/XPT2046SoftSPI/Source/Xpt2046SoftSpi.h
  • Drivers/st7789-i8080-module/CMakeLists.txt
  • Drivers/st7789-i8080-module/LICENSE-Apache-2.0.md
  • Drivers/st7789-i8080-module/README.md
  • Drivers/st7789-i8080-module/bindings/sitronix,st7789-i8080.yaml
  • Drivers/st7789-i8080-module/devicetree.yaml
  • Drivers/st7789-i8080-module/include/bindings/st7789_i8080.h
  • Drivers/st7789-i8080-module/include/drivers/st7789_i8080.h
  • Drivers/st7789-i8080-module/include/st7789_i8080_module.h
  • Drivers/st7789-i8080-module/source/module.cpp
  • Drivers/st7789-i8080-module/source/st7789_i8080.cpp
  • Drivers/xpt2046-softspi-module/CMakeLists.txt
  • Drivers/xpt2046-softspi-module/LICENSE-Apache-2.0.md
  • Drivers/xpt2046-softspi-module/README.md
  • Drivers/xpt2046-softspi-module/bindings/xptek,xpt2046-softspi.yaml
  • Drivers/xpt2046-softspi-module/devicetree.yaml
  • Drivers/xpt2046-softspi-module/include/bindings/xpt2046_softspi.h
  • Drivers/xpt2046-softspi-module/include/drivers/xpt2046_softspi.h
  • Drivers/xpt2046-softspi-module/include/xpt2046_softspi_module.h
  • Drivers/xpt2046-softspi-module/source/module.cpp
  • Drivers/xpt2046-softspi-module/source/xpt2046_softspi.cpp
  • Firmware/Kconfig
  • Modules/lvgl-module/include/tactility/lvgl_pointer.h
  • Modules/lvgl-module/source/lvgl_pointer.c
  • Platforms/platform-esp32/CMakeLists.txt
  • Platforms/platform-esp32/bindings/espressif,esp32-i8080.yaml
  • Platforms/platform-esp32/include/tactility/bindings/esp32_i8080.h
  • Platforms/platform-esp32/include/tactility/drivers/esp32_i8080.h
  • Platforms/platform-esp32/source/drivers/esp32_i8080.cpp
  • Platforms/platform-esp32/source/module.cpp
  • Tactility/Include/Tactility/app/touchcalibration/TouchCalibration.h
  • Tactility/Include/Tactility/settings/TouchCalibrationSettings.h
  • Tactility/Source/Tactility.cpp
  • Tactility/Source/app/kerneldisplay/KernelDisplay.cpp
  • Tactility/Source/app/setup/Setup.cpp
  • Tactility/Source/app/timezone/TimeZone.cpp
  • Tactility/Source/app/touchcalibration/TouchCalibration.cpp
  • Tactility/Source/lvgl/Lvgl.cpp
  • Tactility/Source/settings/TouchCalibrationSettings.cpp
  • TactilityKernel/bindings/i8080-controller.yaml
  • TactilityKernel/bindings/tactility,gpio-hog.yaml
  • TactilityKernel/include/tactility/bindings/gpio_hog.h
  • TactilityKernel/include/tactility/drivers/gpio_hog.h
  • TactilityKernel/include/tactility/drivers/i8080_controller.h
  • TactilityKernel/source/drivers/gpio_hog.cpp
  • TactilityKernel/source/drivers/i8080_controller.cpp
  • TactilityKernel/source/kernel_init.cpp
  • device.py
💤 Files with no reviewable changes (23)
  • Devices/cyd-2432s024r/Source/Configuration.cpp
  • Devices/cyd-e32r28t/Source/devices/Display.h
  • Devices/cyd-2432s028rv3/Source/module.cpp
  • Devices/cyd-2432s028r/Source/Configuration.cpp
  • Devices/cyd-2432s024r/Source/devices/Display.h
  • Devices/cyd-2432s028rv3/Source/Configuration.cpp
  • Devices/cyd-2432s028r/Source/module.cpp
  • Devices/cyd-e32r28t/Source/Configuration.cpp
  • Devices/cyd-2432s028rv3/Source/devices/Display.h
  • Devices/cyd-2432s028r/Source/devices/Display.h
  • Devices/lilygo-thmi/Source/devices/Display.h
  • Devices/cyd-2432s024r/Source/devices/Display.cpp
  • Devices/lilygo-thmi/Source/Configuration.cpp
  • Drivers/XPT2046SoftSPI/Source/Xpt2046SoftSpi.cpp
  • Drivers/XPT2046SoftSPI/Source/Xpt2046SoftSpi.h
  • Devices/lilygo-thmi/Source/devices/Display.cpp
  • Devices/cyd-2432s028r/Source/devices/Display.cpp
  • Devices/lilygo-thmi/Source/Init.cpp
  • Devices/lilygo-thmi/Source/devices/Power.h
  • Devices/cyd-e32r28t/Source/devices/Display.cpp
  • Devices/lilygo-thmi/Source/devices/Power.cpp
  • Devices/cyd-2432s028rv3/Source/devices/Display.cpp
  • Drivers/XPT2046/Source/Xpt2046Touch.h

Comment thread Platforms/platform-esp32/source/drivers/esp32_i8080.cpp
Comment thread TactilityKernel/bindings/tactility,gpio-hog.yaml Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b9002f61-e02b-4d06-a154-aee92e30156d

📥 Commits

Reviewing files that changed from the base of the PR and between f311038 and 1069032.

📒 Files selected for processing (16)
  • Devices/lilygo-thmi/lilygo,thmi.dts
  • Drivers/ili9341-module/bindings/ilitek,ili9341.yaml
  • Drivers/ili9341-module/include/drivers/ili9341.h
  • Drivers/ili9341-module/source/ili9341.cpp
  • Drivers/st7789-i8080-module/bindings/sitronix,st7789-i8080.yaml
  • Drivers/st7789-i8080-module/include/drivers/st7789_i8080.h
  • Drivers/st7789-i8080-module/source/st7789_i8080.cpp
  • Drivers/st7789-module/bindings/sitronix,st7789.yaml
  • Drivers/st7789-module/include/drivers/st7789.h
  • Drivers/st7789-module/source/st7789.cpp
  • Platforms/platform-esp32/include/tactility/drivers/esp32_i8080.h
  • Platforms/platform-esp32/source/drivers/esp32_i8080.cpp
  • Tactility/Include/Tactility/app/touchcalibration/TouchCalibration.h
  • TactilityKernel/bindings/tactility,gpio-hog.yaml
  • TactilityKernel/include/tactility/drivers/gpio_hog.h
  • TactilityKernel/source/drivers/gpio_hog.cpp
🚧 Files skipped from review as they are similar to previous changes (8)
  • Drivers/st7789-i8080-module/bindings/sitronix,st7789-i8080.yaml
  • TactilityKernel/source/drivers/gpio_hog.cpp
  • Drivers/st7789-i8080-module/include/drivers/st7789_i8080.h
  • Tactility/Include/Tactility/app/touchcalibration/TouchCalibration.h
  • Devices/lilygo-thmi/lilygo,thmi.dts
  • Platforms/platform-esp32/include/tactility/drivers/esp32_i8080.h
  • Platforms/platform-esp32/source/drivers/esp32_i8080.cpp
  • Drivers/st7789-i8080-module/source/st7789_i8080.cpp

Comment thread Drivers/ili9341-module/bindings/ilitek,ili9341.yaml

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6edf8067-e3cc-4deb-8991-465cbdc30f14

📥 Commits

Reviewing files that changed from the base of the PR and between f1e96d2 and daded51.

📒 Files selected for processing (4)
  • Devices/cyd-e32r32p/CMakeLists.txt
  • Devices/cyd-e32r32p/cyd,e32r32p.dts
  • Devices/cyd-e32r32p/device.properties
  • Devices/cyd-e32r32p/devicetree.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • Devices/cyd-e32r32p/device.properties

Comment thread Devices/cyd-e32r32p/cyd,e32r32p.dts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Devices/unphone/Source/InitBoot.cpp (1)

4-4: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add <tactility/check.h> (or keep the umbrella include) check() is not provided by the remaining headers; LOG_I/LOG_E and the device APIs already are.

🧹 Nitpick comments (2)
Buildscripts/DevicetreeCompiler/source/generator.py (1)

121-139: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider gating on type == "int" explicitly.

The function works correctly today (non-numeric values are safely skipped via the try/except), but explicitly checking binding_property.type before attempting the int parse would make the "only applies to int-typed properties" contract in the docstring self-enforcing rather than implicit.

Drivers/hx8357-module/source/hx8357.cpp (1)

396-406: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Minor: name the remaining magic opcodes for consistency.

0x28 (DISPOFF) and 0x10 (SLPIN) are inlined with comments while every other opcode is a named constexpr alongside the others at the top of the file.

♻️ Proposed fix
 constexpr uint8_t HX8357_SLPOUT = 0x11;
+constexpr uint8_t HX8357_SLPIN = 0x10;
 constexpr uint8_t HX8357_INVOFF = 0x20;
...
 constexpr uint8_t HX8357_DISPON = 0x29;
+constexpr uint8_t HX8357_DISPOFF = 0x28;

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8bf64dcc-6603-499e-8a29-2eb27ef993ee

📥 Commits

Reviewing files that changed from the base of the PR and between daded51 and 9f92c6d.

📒 Files selected for processing (46)
  • Buildscripts/DevicetreeCompiler/source/binding_parser.py
  • Buildscripts/DevicetreeCompiler/source/generator.py
  • Buildscripts/DevicetreeCompiler/source/models.py
  • Buildscripts/DevicetreeCompiler/tests/test_integration.py
  • Devices/cyd-e32r32p/cyd,e32r32p.dts
  • Devices/elecrow-crowpanel-basic-28/CMakeLists.txt
  • Devices/elecrow-crowpanel-basic-28/Source/Configuration.cpp
  • Devices/elecrow-crowpanel-basic-28/Source/devices/Display.cpp
  • Devices/elecrow-crowpanel-basic-28/Source/devices/Display.h
  • Devices/elecrow-crowpanel-basic-28/device.properties
  • Devices/elecrow-crowpanel-basic-28/devicetree.yaml
  • Devices/elecrow-crowpanel-basic-28/elecrow,crowpanel-basic-28.dts
  • Devices/elecrow-crowpanel-basic-28/source/module.cpp
  • Devices/unphone/CMakeLists.txt
  • Devices/unphone/Source/Configuration.cpp
  • Devices/unphone/Source/InitBoot.cpp
  • Devices/unphone/Source/devices/Hx8357Display.cpp
  • Devices/unphone/Source/devices/Hx8357Display.h
  • Devices/unphone/Source/devices/Touch.cpp
  • Devices/unphone/Source/devices/Touch.h
  • Devices/unphone/Source/hx8357/README.md
  • Devices/unphone/Source/hx8357/disp_spi.c
  • Devices/unphone/Source/hx8357/disp_spi.h
  • Devices/unphone/Source/hx8357/hx8357.c
  • Devices/unphone/Source/hx8357/hx8357.h
  • Devices/unphone/Source/lvgl_spi_conf.h
  • Devices/unphone/devicetree.yaml
  • Devices/unphone/unphone.dts
  • Drivers/XPT2046/CMakeLists.txt
  • Drivers/XPT2046/README.md
  • Drivers/XPT2046/Source/Xpt2046Touch.cpp
  • Drivers/XPT2046/Source/Xpt2046Touch.h
  • Drivers/hx8357-module/CMakeLists.txt
  • Drivers/hx8357-module/LICENSE-Apache-2.0.md
  • Drivers/hx8357-module/README.md
  • Drivers/hx8357-module/bindings/himax,hx8357.yaml
  • Drivers/hx8357-module/devicetree.yaml
  • Drivers/hx8357-module/include/bindings/hx8357.h
  • Drivers/hx8357-module/include/drivers/hx8357.h
  • Drivers/hx8357-module/include/hx8357_module.h
  • Drivers/hx8357-module/source/hx8357.cpp
  • Drivers/hx8357-module/source/module.cpp
  • Drivers/ili9341-module/bindings/ilitek,ili9341.yaml
  • Drivers/ili9488-module/bindings/ilitek,ili9488.yaml
  • Drivers/st7789-i8080-module/bindings/sitronix,st7789-i8080.yaml
  • Drivers/st7789-module/bindings/sitronix,st7789.yaml
💤 Files with no reviewable changes (17)
  • Drivers/XPT2046/README.md
  • Devices/unphone/Source/lvgl_spi_conf.h
  • Drivers/XPT2046/CMakeLists.txt
  • Drivers/XPT2046/Source/Xpt2046Touch.h
  • Devices/elecrow-crowpanel-basic-28/Source/devices/Display.h
  • Devices/unphone/Source/hx8357/hx8357.h
  • Devices/unphone/Source/hx8357/disp_spi.h
  • Devices/unphone/Source/hx8357/README.md
  • Devices/unphone/Source/devices/Touch.cpp
  • Devices/elecrow-crowpanel-basic-28/Source/devices/Display.cpp
  • Devices/elecrow-crowpanel-basic-28/Source/Configuration.cpp
  • Devices/unphone/Source/devices/Touch.h
  • Devices/unphone/Source/hx8357/disp_spi.c
  • Devices/unphone/Source/hx8357/hx8357.c
  • Devices/unphone/Source/devices/Hx8357Display.cpp
  • Devices/unphone/Source/devices/Hx8357Display.h
  • Drivers/XPT2046/Source/Xpt2046Touch.cpp
🚧 Files skipped from review as they are similar to previous changes (3)
  • Drivers/st7789-i8080-module/bindings/sitronix,st7789-i8080.yaml
  • Drivers/ili9341-module/bindings/ilitek,ili9341.yaml
  • Devices/cyd-e32r32p/cyd,e32r32p.dts

Comment on lines +6 to +8
extern "C" {

extern Driver hx8357_driver;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu
rg -n -C2 'extern "C"|Driver hx8357_driver' \
  Drivers/hx8357-module/source/module.cpp \
  Drivers/hx8357-module/source/hx8357.cpp

Repository: TactilityProject/Tactility

Length of output: 1048


🌐 Web query:

C++ extern "C" variable declaration definition outside extern "C" same linkage object declaration language linkage

💡 Result:

In C++, language linkage (such as extern "C") is a property of a name or entity that dictates how the compiler handles its name in the generated object code, primarily to facilitate interoperability with other languages like C [1][2]. Key rules regarding variable declarations with extern "C": 1. Impact of Language Linkage: The extern "C" specifier applies to function types, function names with external linkage, and variable names with external linkage [3][4][5]. It instructs the compiler to use C-style naming conventions (typically avoiding name mangling), which is critical for linking with C modules [6][2]. 2. Declaration vs. Definition: A declaration inside an extern "C" block is treated as if it contains the extern specifier for the purposes of determining linkage and whether it is a definition [3]. Consequently, declaring a variable within an extern "C" block that does not include an initializer is generally treated as a declaration, not a definition [3]. 3. Linkage consistency: If a variable is declared both inside and outside an extern "C" block, or in different translation units, the declarations must be consistent [4][5]. Specifically, if an entity is declared with C language linkage, it must have that linkage consistently [4][5]. Two declarations for a variable with C language linkage with the same name (ignoring namespace qualifications) refer to the same variable [4][5]. 4. Definition Constraints: There can be at most one definition for an entity with a given name with C language linkage in the entire program [4][5]. If you declare a variable with extern "C" in one place, it implicitly has external linkage [7]. If you later provide a definition for that variable, it must match the expected C language linkage [4][5]. In practice, if you declare extern "C" int x; and later define int x = 0; (without the extern "C" block), you may encounter issues because the second declaration would typically be interpreted as having C++ language linkage, creating a conflict with the first declaration's C language linkage [3][4]. To ensure correct behavior, the definition should also be wrapped in or declared with the appropriate language linkage to maintain consistency across the program [4][5].

Citations:


🌐 Web query:

cppreference C++ language linkage object declaration definition extern "C" variable same linkage

💡 Result:

In C++, language linkage (e.g., extern "C") and object linkage (storage class) are distinct but related concepts [1][2]. Language Linkage Language linkage determines requirements like calling conventions and name mangling to enable interoperability between different programming languages [1][3]. By default, entities in C++ have "C++" language linkage [4][5]. Using extern "C" changes this to "C" language linkage [1]. Declaration vs. Definition inside Language Linkage A key rule is that a declaration directly contained within a language linkage specification (such as extern "C" { declaration; }) is treated as if it were declared with the extern storage class specifier [1][3]. This affects whether the declaration is considered a definition [1][4]. 1. Declaration as Definition: If you declare a variable inside an extern "C" block without an explicit extern specifier, the language linkage specification's rule forces it to be treated as if it were extern [1][4]. Consequently, it is treated as a declaration (not a definition) unless it is initialized [1][3]. Example: extern "C" { int x; } // Treated as: extern "C" extern int x; (Declaration) 2. Defining a Variable with extern "C": To provide a definition for a variable with "C" language linkage, you typically declare it as extern "C" and then provide an initialization in a translation unit [1]. If you use extern "C" { int x = 0; }, the initialization makes it a definition despite the implicit extern treatment [1]. Same Linkage Requirements All declarations of an entity must agree on their language linkage [3]. If two declarations of the same name have conflicting language linkages, the program is ill-formed [3]. Furthermore, a redeclaration of an entity without an explicit language linkage specification inherits the language linkage of the original declaration [3]. In summary, extern "C" does not change the storage class (linkage) of the variable directly; rather, the syntax of the linkage specification block forces the included declarations to act as if they have the extern storage class, impacting whether the compiler treats them as definitions [1][3]. Top Results: [1], [4], [3], [2]

Citations:


🌐 Web query:

cppreference redeclaration inherits language linkage variable extern "C" different translation unit object name mangling

💡 Result:

In C++, language linkage (e.g., extern "C") affects both name mangling and the calling convention of functions, and for variables, it primarily affects the name mangling [1][2]. If two declarations of the same entity give it different language linkages, the program is ill-formed [1][2]. However, if these declarations appear in different translation units (TUs), no diagnostic is required, meaning the compiler/linker is not obligated to report the error [1][2]. The critical implications are as follows: 1. Name Mangling: Entities with C language linkage use C-style name mangling (typically no mangling or simple prefix/suffix depending on the platform) [1][3]. Entities with the default C++ language linkage undergo C++ name mangling, which incorporates type information into the symbol name [1][2]. If a variable is declared as extern "C" in one TU and as C++ linkage in another, the linker will see two different symbols for what the source code treats as the same entity. This typically leads to a linker error (e.g., undefined reference) because the linker cannot reconcile the two different mangled names [3]. 2. Redeclaration Rules: A redeclaration of an entity without an explicit linkage specification inherits the language linkage of the previous declaration [1]. When you explicitly provide a different language linkage in a redeclaration, you create a conflict [2]. 3. Different Translation Units: While the standard deems the program ill-formed if these declarations conflict, the lack of a required diagnostic for cross-TU mismatches means that the program may exhibit undefined behavior at link time or run time, as the two TUs will not successfully link to the same memory location or will encounter symbol collisions [1][2]. In summary, you cannot validly declare a variable with different language linkages across translation units [1][2]. Doing so violates the One Definition Rule (ODR) and creates symbol name mismatches that prevent correct linking [1][2][4].

Citations:


Match hx8357_driver linkage in both translation units. The declaration in module.cpp gives it C linkage, but Drivers/hx8357-module/source/hx8357.cpp defines it with C++ linkage, so the linker will look for different symbols. Wrap the definition in extern "C" or move the declaration/definition to a shared header with matching linkage.

Source: MCP tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant