Update USER_SETTINGS.cpp #717
Conversation
Improve configuration file structure and readability - Grouped settings logically (CAN, WiFi, Web Server, MQTT, Charger, etc.) - Enhanced comments for clarity and better documentation - Standardized formatting for consistent style and readability - Added floating-point suffix (`f`) for explicit type usage in charger settings - Prepared code for future validation and error handling This improves maintainability and makes the configuration easier to understand and manage.
| CAN_ADDON_MCP2515 = Add-on CAN MCP2515 connected to GPIO pins | ||
| CANFD_ADDON_MCP2518 = Add-on CAN-FD MCP2518 connected to GPIO pins | ||
| */ | ||
| /** |
There was a problem hiding this comment.
This comment change makes it HARDER to use the software, when the examples are removed.
It also removes the most important comment, that specifies the CAN mappings.
I suggest reverting all of it. There is no improvement here.
| .battery_double = CAN_ADDON_MCP2515, // (OPTIONAL) Which CAN is your second battery connected to? | ||
| .charger = CAN_NATIVE // (OPTIONAL) Which CAN is your charger connected to? | ||
| .battery = CAN_NATIVE, // Battery CAN interface | ||
| .inverter = CAN_NATIVE, // Inverter CAN interface (use RS485 if not configured) |
There was a problem hiding this comment.
It does not use RS485 automatically, bad wording. Original wording makes it more clear.
| "be_"; // Custom prefix for MQTT object ID. Previously, the prefix was automatically set to "esp32-XXXXXX_" | ||
| const char* mqtt_device_name = | ||
| "Battery Emulator"; // Custom device name in Home Assistant. Previously, the name was automatically set to "BatteryEmulator_esp32-XXXXXX" | ||
| const char* mqtt_topic_name = "BE"; // Custom MQTT topic name |
There was a problem hiding this comment.
These comments are VITAL for people updating from older versions. They should not be removed, since it makes using MQTT way harder.
| volatile float CHARGER_MAX_A = 11.5; // Max current output (amps) of charger | ||
| volatile float CHARGER_END_A = 1.0; // Current at which charging is considered complete | ||
| /* ------------------------- Charger Settings ------------------------- */ | ||
| /* Charger settings for optional generator charging. */ |
There was a problem hiding this comment.
While we are editing this comment, would be good to further clarify that Charger section is completely unused if no optional charger hardware has been configured. Not sure how to word it better...
|
Thanks for the contribution 🙌 . I put in a few comments, to pass the pre-commit checks, also see the Contributing guide for how to do the clang formatting to get the builds to pass. |
Title
Refactor Configuration File for Improved Readability and Maintainability
Description
This pull request refactors the configuration file to enhance its readability, organization, and future maintainability. Key updates include:
Logical Grouping:
Improved Comments:
Consistent Formatting:
Type Explicitness:
f) for charger-related settings to make the data types explicit.Future Proofing:
How I Tested
USER_SETTINGS.handUSER_SECRETS.hare correctly applied.Key Changes
CAN Configuration.WiFi Settings,Web Server Settings, andMQTT Settings.Charger Settingswith explicit type declarations.Possible Follow-Ups
Request for Review
Please review the following:
Feedback and suggestions are greatly appreciated. Thank you! 🙌