Kernel and TactilitySDK improvements

[KenVanHoeylandt]

Summary by CodeRabbit

• New Features

  • Expanded public device/driver/module APIs and added a composite module start helper.
  • Added a kernel symbol registry and new exported symbols (graphics, C++ nothrow, I2S, extra math functions).

• Refactor

  • Unified and renamed device traversal APIs for consistency.
  • Converted many inline helpers to explicit public declarations.

• Chores

  • Replaced several shell-based release scripts with Python-based SDK release tooling.

• Style

  • Header/name consistency fixes.

[coderabbitai]

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Shell-based SDK release tooling was replaced with Python scripts: added Buildscripts/release-sdk.py and Buildscripts/release-sdk-current.py and removed several shell helpers. ESP-IDF component registration and prebuilt-library linkage were moved into Buildscripts/TactilitySDK/CMakeLists and removed from Buildscripts/CMake/CMakeLists.txt. Device iteration APIs were renamed from for_each_* to device_* and many former inline device/driver accessors were exposed as public functions. New driver/module lifecycle functions, a kernel symbol registry (KERNEL_SYMBOLS), root-module initialization, and additional exported symbols were added. Minor include and call-site name updates across source and test files.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 3.13% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Kernel and TactilitySDK improvements' is overly vague and generic. It does not convey the specific nature of the changes, such as the refactoring of device/driver APIs, build system reorganization, or the migration from shell to Python scripts.	Consider a more descriptive title that highlights the primary changes, such as 'Refactor device/driver APIs and migrate build scripts to Python' or 'Restructure kernel device management and SDK release automation'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ 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

---

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

[coderabbitai]

Actionable comments posted: 6

Caution

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

⚠️ Outside diff range comments (1)

TactilityKernel/Source/device.cpp (1)

297-304: ⚠️ Potential issue | 🟠 Major

Potential thread safety issue: device_for_each_child lacks synchronization.

Unlike device_for_each and device_for_each_of_type which acquire ledger_lock, this function iterates over data->children without holding the device lock. Meanwhile, device_add_child and device_remove_child (lines 72-88) both acquire device_lock when modifying the children vector.

If another thread adds/removes a child during iteration, this could cause undefined behavior (iterator invalidation).

🔒 Proposed fix to add device locking

 void device_for_each_child(Device* device, void* callbackContext, bool(*on_device)(struct Device* device, void* context)) {
+    device_lock(device);
     auto* data = get_device_private(device);
     for (auto* child_device : data->children) {
         if (!on_device(child_device, callbackContext)) {
             break;
         }
     }
+    device_unlock(device);
 }

🧹 Nitpick comments (5)

TactilityKernel/Source/module.cpp (1)

72-78: Consider cleanup on partial failure.

If module_add succeeds but module_start fails, the module remains in the ledger in a non-started state. Depending on the intended use case, you may want to call module_remove on failure to maintain consistency. If this is intentional (allowing retry of module_start later), consider documenting this behavior in the header.

♻️ Optional: Add rollback on start failure

 error_t module_construct_add_start(struct Module* module) {
     error_t error = module_construct(module);
     if (error != ERROR_NONE) return error;
     error = module_add(module);
     if (error != ERROR_NONE) return error;
-    return module_start(module);
+    error = module_start(module);
+    if (error != ERROR_NONE) {
+        module_remove(module);
+        return error;
+    }
+    return ERROR_NONE;
 }

TactilityKernel/Source/kernel_init.cpp (2)

29-62: Consider rollback on partial initialization failure.

The kernel_init function initializes modules sequentially but doesn't roll back previously initialized modules if a later step fails. For example, if platform_module init fails (line 37-40), root_module remains initialized. This could leave the system in a partially initialized state.

If this is intentional (e.g., partial initialization is acceptable or the system halts on error anyway), this is fine. Otherwise, consider adding cleanup logic.

---

18-20: Empty stop function is acceptable but could use a brief comment.

The stop() function currently does nothing. If this is intentional because root drivers are never stopped during normal operation, a brief comment explaining this would help future maintainers understand the design decision.

TactilityKernel/Include/tactility/device.h (1)

233-248: Minor: Inconsistent parameter naming convention.

The iteration functions use different naming styles:

• device_for_each: uses callback_context (snake_case)
• device_for_each_child and device_for_each_of_type: use callbackContext (camelCase)

Consider standardizing to one style for API consistency.

Buildscripts/release-sdk.py (1)

113-116: Avoid appending to idf-version.txt across runs.

Append mode can accumulate duplicate version strings if the script is re-run on the same directory.

🔧 Proposed fix

-    with open(os.path.join(target_path, "idf-version.txt"), "a") as f:
+    with open(os.path.join(target_path, "idf-version.txt"), "w") as f:
         f.write(esp_idf_version)

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

TactilityKernel/Source/kernel_symbols.c (1)

109-112: Consider adding a brief comment explaining the ESP_PLATFORM exclusion.

The conditional exclusion of log_generic for ESP_PLATFORM is not immediately obvious. A short comment explaining why this symbol is unavailable on ESP (e.g., different logging backend, linking constraints) would improve maintainability.

📝 Suggested documentation

     // log
+    // log_generic is not available on ESP_PLATFORM due to <reason>
 `#ifndef` ESP_PLATFORM
     DEFINE_MODULE_SYMBOL(log_generic),
 `#endif`

Buildscripts/release-sdk.py (1)

6-6: Remove unused subprocess import.

The subprocess module is imported but never used in this file.

🔧 Proposed fix

 import os
 import shutil
 import glob
-import subprocess
 import sys