Repository navigation
Export location module and implement compass API - #725
Conversation
Walkthrough
Priority: Merge Risk:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🔇 Additional comments: 1
TactilityKernel/source/drivers/compass.cpp: 1
25-25:Trivial · Maintainability & Code Quality · Quick win
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.Use a standard C++ constant for π.
M_PIis an extension and is not guaranteed by<cmath>. This project selects C++23, so<numbers>andstd::numbers::pi_v<float>provide the constant without depending on a platform macro. This is a portability change; the supplied build configuration does not establish a current build failure. Based on learnings, “M_PI is a non-standard extension” and reviewers should flag its use in portable code. (gnu.org)Source: Learnings

Review info
-
Run configuration-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
3e15e9f0-cb6e-4bc6-aebe-ea7276840d85
Files selected for processing: 4-
TactilityKernel/include/tactility/drivers/compass.hTactilityKernel/source/drivers/compass.cppTactilityKernel/source/symbols.cTactilityKernel/tests/source/compass_test.cpp
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
-
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

Major · Connect location-module to the standard SDK integration. · release-sdk-modules.txt:8
Buildscripts/release-sdk-modules.txt:8
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winConnect
location-moduleto the standard SDK integration.The release list copies the module, but ordinary SDK apps depend only on
TactilitySDK. The ESP32 integration does not select or requirelocation-module, and the POSIX integration does not expose its headers. Apps using the standard templates therefore cannot use the location API without manual CMake configuration.Suggested fix
diff --git a/Buildscripts/TactilitySDK/CMakeLists.txt b/Buildscripts/TactilitySDK/CMakeLists.txt @@ - REQUIRES esp_timer app-module crypt-module gps-module lvgl-module lvgl-window-manager-module service-module + REQUIRES esp_timer app-module crypt-module gps-module location-module lvgl-module lvgl-window-manager-module service-module diff --git a/Buildscripts/TactilitySDK/TactilitySDK.esp32.cmake b/Buildscripts/TactilitySDK/TactilitySDK.esp32.cmake @@ crypt-module gps-module + location-module lvgl-module diff --git a/Buildscripts/TactilitySDK/TactilitySDK.posix.cmake b/Buildscripts/TactilitySDK/TactilitySDK.posix.cmake @@ ${TACTILITY_SDK_PATH}/Modules/gps-module/include + ${TACTILITY_SDK_PATH}/Modules/location-module/include ${TACTILITY_SDK_PATH}/Modules/lvgl-module/include

Review info
-
Run configuration-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
a4713428-0d74-4d74-b313-bb778e501e40
Files selected for processing: 3-
TactilityKernel/include/tactility/drivers/compass.hTactilityKernel/source/drivers/compass.cppTactilityKernel/tests/source/compass_test.cpp
Files skipped from review as they are similar to previous changes: 3-
- TactilityKernel/tests/source/compass_test.cpp
- TactilityKernel/source/drivers/compass.cpp
- TactilityKernel/include/tactility/drivers/compass.h
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.
-
Summary by CodeRabbit