Tab5 audio, I2C improvements, UiDensity moved to lvgl-module and cleanup

[KenVanHoeylandt]

• UiDensity moved to lvgl-module
• Deleted tt_hal and tt_hal_gpio (breaks apps, but will fix those right after merging)
• Added I2C 8 bit register operations
• Added device.properties to simulator
• Improved Tab5 hardware init, implement audio
• Add README.md to kernel

Summary by CodeRabbit

• New Features

  • Configurable UI density (compact mode) across many devices, exposed to LVGL-based UI
  • New I2C register operations for finer bit-level hardware control
  • Added a Simulator device profile

• Improvements

  • More robust Tab5 hardware initialization with improved power/audio startup handling
  • Enhanced device/property parsing and lookup with validation for build-time configuration

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds CMake property-parsing helpers and reads uiDensity from device.properties for multiple devices. Removes UiDensity from the HAL Configuration and deletes related C ABI wrappers. Introduces LVGL-facing compile definitions and a new function lvgl_get_ui_density() with updated public macros. Replaces HAL-based density checks across many UI sources to use the LVGL API. Adds I2C register8 operations to the kernel driver API and symbol table. Implements I2S and I2C controller-based power/sound initialization and device tree updates for the M5Stack Tab5.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 2.63% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the three main changes: Tab5 audio improvements, I2C enhancements, and moving UiDensity from HAL to lvgl-module with related cleanup.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch develop

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

Devices/m5stack-tab5/Source/Configuration.cpp (1)

69-71: initPower discards the actual error and always returns ERROR_UNDEFINED.

The error variable already holds the specific error from i2c_controller_write_register_array, but the function returns a generic ERROR_UNDEFINED instead. In contrast, initSound correctly propagates the real error via return error. Consider returning error here as well for consistent and more informative error reporting.

Suggested diff

     auto error = i2c_controller_write_register_array(i2c_controller, IO_EXPANDER1_ADDRESS, reg_data_io1_1, sizeof(reg_data_io1_1), pdMS_TO_TICKS(100));
     if (error != ERROR_NONE) {
         LOG_E(TAG, "IO expander 1 init failed in phase 1");
-        return ERROR_UNDEFINED;
+        return error;
     }
 
     constexpr auto IO_EXPANDER2_ADDRESS = 0x44;
     error = i2c_controller_write_register_array(i2c_controller, IO_EXPANDER2_ADDRESS, reg_data_io2, sizeof(reg_data_io2), pdMS_TO_TICKS(100));
     if (error != ERROR_NONE) {
         LOG_E(TAG, "IO expander 2 init failed");
-        return ERROR_UNDEFINED;
+        return error;
     }
 
     // The M5Stack code applies this, but it's not known why
     // TODO: Remove and test it extensively
     tt::kernel::delayTicks(10);
 
     error = i2c_controller_write_register_array(i2c_controller, IO_EXPANDER1_ADDRESS, reg_data_io1_2, sizeof(reg_data_io1_2), pdMS_TO_TICKS(100));
     if (error != ERROR_NONE) {
         LOG_E(TAG, "IO expander 1 init failed in phase 2");
-        return ERROR_UNDEFINED;
+        return error;
     }

Also applies to: 76-78, 86-88

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (3)

Devices/m5stack-tab5/Source/Configuration.cpp (1)

145-145: Nit: trailing whitespace before the comma.

-    error = i2c_controller_register8_set_bits(i2c_controller, IO_EXPANDER1_ADDRESS, AMP_REGISTER, 0b00000010 , pdMS_TO_TICKS(100));
+    error = i2c_controller_register8_set_bits(i2c_controller, IO_EXPANDER1_ADDRESS, AMP_REGISTER, 0b00000010, pdMS_TO_TICKS(100));

Buildscripts/properties.cmake (2)

52-52: Flat-list map: value-as-key collision risk with list(FIND ...).

The map is stored as a flat alternating list [key0, val0, key1, val1, ...]. list(FIND ...) returns the index of the first occurrence of the string in the entire list — it doesn't distinguish keys from values. If a value string happens to equal a subsequent key string, GET_VALUE_FROM_MAP will return incorrect results.

For the current use case (device.properties with simple values like "compact" and section-prefixed keys like [lvgl]uiDensity), collisions are extremely unlikely. Just calling it out as a latent limitation.

A safer alternative would be to use CMake's CACHE variables or a naming-convention approach (e.g., set(MAP_${KEY} "${VALUE}")) for true O(1) lookups without collision risk.

Also applies to: 59-69

---

40-41: Section brackets are included in the stored key prefix.

The section line (e.g., [lvgl]) is stored verbatim as the prefix, so callers must look up keys as [lvgl]uiDensity rather than lvgl.uiDensity or similar. This is fine as long as it's a deliberate convention — just noting it for documentation clarity.