Fixed complier warnings and errors #439
Conversation
📝 WalkthroughWalkthroughHeader Configuration.h now includes the C++ header. UI formatting specifiers were adjusted: GPS changed a baudRate specifier to %u; I2C and power code use fixed-width macros (PRIu32/PRId32) for frequency, voltage, and current formatting. URL utilities replaced sprintf with snprintf for hex-encoding and changed sscanf's format to scan into size_t ("%zx") during decoding. Control flow and public APIs were not modified; changes are limited to format specifiers and a buffer-safety call. Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-10-26T12:44:21.858ZApplied to files:
🔇 Additional comments (1)
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: 0
🧹 Nitpick comments (4)
Tactility/Source/app/gpssettings/GpsSettings.cpp (1)
126-126: Format specifier fix looks correct.The change from
%luto%ucorrectly addresses the format specifier mismatch forbaudRate. On ESP32 (32-bit platform), this is the appropriate specifier.For maximum portability across platforms, consider using
PRIu32from<inttypes.h>ifbaudRateisuint32_t:lv_label_set_text_fmt(baud_label, "Baud: %" PRIu32, configuration.baudRate);Tactility/Source/app/i2csettings/I2cSettings.cpp (1)
69-69: Format specifier fix looks correct.The change from
%luto%uaddresses the format specifier mismatch forclk_speed. This aligns with the ESP32 platform's type definitions.For maximum portability, if
clk_speedisuint32_t, consider usingPRIu32from<inttypes.h>:lv_label_set_text_fmt(frequency_label, "Frequency: %" PRIu32 " Hz", configuration.config.master.clk_speed);Tactility/Source/app/power/Power.cpp (2)
117-117: Format specifier fix looks correct.The change from
%luto%uproperly matches theuint32_ttype ofbattery_voltage(line 96) on the ESP32 platform.For maximum portability, consider using
PRIu32from<inttypes.h>:lv_label_set_text_fmt(batteryVoltageLabel, "Battery voltage: %" PRIu32 " mV", battery_voltage);
129-129: Format specifier fix looks correct.The change from
%ldto%dproperly matches theint32_ttype ofcurrent(line 89) on the ESP32 platform.For maximum portability, consider using
PRId32from<inttypes.h>:lv_label_set_text_fmt(currentLabel, "Current: %" PRId32 " mAh", current);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Tactility/Include/Tactility/hal/uart/Configuration.hTactility/Source/app/gpssettings/GpsSettings.cppTactility/Source/app/i2csettings/I2cSettings.cppTactility/Source/app/power/Power.cppTactility/Source/network/Url.cpp
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: KenVanHoeylandt
Repo: ByteWelder/Tactility PR: 406
File: Buildscripts/CDN/devices.properties:0-0
Timestamp: 2025-11-02T15:49:07.895Z
Learning: In the ByteWelder/Tactility repository, alphabetical ordering issues (such as entries in Buildscripts/CDN/devices.properties) and similar organizational/formatting concerns should be marked as "minor" severity rather than "major" severity.
Learnt from: KenVanHoeylandt
Repo: ByteWelder/Tactility PR: 391
File: Boards/CYD-2432S028RV3/Source/devices/Display.h:11-19
Timestamp: 2025-10-26T12:44:21.858Z
Learning: In the Tactility codebase, prefer `constexpr auto` for constant declarations (such as LCD pin configurations, resolutions, and buffer sizes) rather than explicit type annotations like `gpio_num_t`, `spi_host_device_t`, `uint16_t`, or `size_t`.
📚 Learning: 2025-10-26T12:44:21.858Z
Learnt from: KenVanHoeylandt
Repo: ByteWelder/Tactility PR: 391
File: Boards/CYD-2432S028RV3/Source/devices/Display.h:11-19
Timestamp: 2025-10-26T12:44:21.858Z
Learning: In the Tactility codebase, prefer `constexpr auto` for constant declarations (such as LCD pin configurations, resolutions, and buffer sizes) rather than explicit type annotations like `gpio_num_t`, `spi_host_device_t`, `uint16_t`, or `size_t`.
Applied to files:
Tactility/Source/app/i2csettings/I2cSettings.cppTactility/Source/app/gpssettings/GpsSettings.cpp
📚 Learning: 2025-10-26T12:40:47.243Z
Learnt from: KenVanHoeylandt
Repo: ByteWelder/Tactility PR: 391
File: Boards/ElecrowCrowpanelAdvance28/Source/devices/Display.cpp:37-43
Timestamp: 2025-10-26T12:40:47.243Z
Learning: The Tactility project uses 80 MHz SPI pixel clock frequency for ST7789 displays, which has been tested and confirmed to work stably on all supported devices (CYD variants, Elecrow CrowPanel, LilyGo, M5Stack, Waveshare boards).
Applied to files:
Tactility/Source/app/i2csettings/I2cSettings.cpp
🔇 Additional comments (3)
Tactility/Source/network/Url.cpp (2)
51-51: Excellent safety improvement.Replacing
sprintfwithsnprintfprevents potential buffer overflow. The buffer size of 10 bytes is sufficient for the format string"%%%02X"(max 3 characters plus null terminator).
72-72: Correct format specifier forsize_t.The change from
%xto%zxproperly matches thesize_ttype ofconversion_buffer(declared at line 62), fixing a format specifier mismatch that would cause warnings or incorrect behavior on 64-bit platforms.Tactility/Include/Tactility/hal/uart/Configuration.h (1)
3-3: Necessary include added.The
#include <string>is required for thestd::string namemember used in both ESP_PLATFORM and non-ESP_PLATFORM configurations. This fixes a missing header compilation error.
1 participant
Fixed some formatting specifier issues, also in
Configuration.han include for<string>appears to be missing.