SensESPAppOptions - allows configuration of SensESPApp application #142
Conversation
- new classes SKListener, SKValueListener<T> and DigitalOutput
Changed period to ListenDelay where required. Added new short-cut types: SKFloatListener, SKIntListener, SKBoolListener and SKStringListener Added new class ThresholdTransform that allows to change one value type to other. Added helper classes NumericThreshold and IntegerThreshold for simple ThresholdTransform setup.
ThresholdTransform - removed this->, debug messages, renamed json "in-range" to "in_range". NumericThreshold - added json schema descriptions, added class comment IntegerThreshold - added json schema descriptions, added class comment
Tested on my environmental sensor and it works.
Http_server - added more information to /info page (MAC, hostname, configuration, etc.) Networking - now configuration scheme is readonly if wifi config is from main.cpp ws_client - now configuration scheme is readonly if server address config is from main.cpp
…in setup (readonly)
WSClient - added readonly label if config is from main.cpp WSClient - fixed issue with not loading configuration from flash when using main.cpp config -> token not loaded from flash
relay_control sample - updated sample to match options changes Networking.cpp - removed reference to Options class all information is passed by ctor arguments led_blinker.cpp - removed reference to Options class all information is passed by ctor arguments
SensApp fixed hostname issue when hostname was already in config json file, but it was overriden in main.cpp
There was a problem hiding this comment.
There's a lot going on in this PR, so I want to test it all before I merge it. And I don't want to test it all before you might make any changes as a result of my comments. So, read all the comments, reply to them, and we'll go from there. (Please be sure to reply "inline" - right after each comment, and NOT at the end of the "Conversation") Thanks so much - this was LOT of work!
sensesp_app_options.cpp - changed LED ws_client.cpp - corrected debug messages
There was a problem hiding this comment.
Please see my individual comments.
Also, could you reformat the files to conform to the project formatting style (Google C++ Style)? The formatting style is defined in .clang-format and VS Code should pick it up automatically from there, so just applying the "Reformat Document" command for each of the changed files should be enough.
|
One more design consideration: if you used the builder pattern as discussed on Slack, you could use the |
|
@mairas that was my initial approach, but problem with this is the app global pointer is set by user in main.cpp and I can't be sure when the user will do it and I didn't want to change this as it would be breaking change. Another option is to create library wide global pointer to app object that will be set in SensESPApp ctor, then I'll be sure when it's set to current app. |
I don't think you need to use that global pointer during initialization. The only need I see for that is for the |
Removed upload speed from platformio.ini SensESPAppOptions - made isWifiSet and IsServerSet based on String.isEmpty() HttpServer - removed reference to SensESPAppOptions in ctor - using global pointer for options Networking - changed "set from main.cpp" to "set from SensespAppOptions" WSClient - removed reference to SensESPAppOptions in ctor
|
New commit addresses all requested changes, I've formatted all changed files as @mairas requested. Just let me know if you spot anything else prior merging the changes. Thanks! |
…ostname isn't set form options
|
Thoroughly tested with ESP8266. One issue found with configuring hostname in main.cpp, but not configuring ssid and password in main.cpp (or vice versa), but after disccusion with @JohnySeven, we decided to fix that in a new issue after this PR is merged. Compiles with ESP32, but the known problem with mDNS is preventing further testing at the moment. Will go ahead and merge, then address that issue, then continue testing. |
This PR was highly discussed and tested with @mairas and @ba58smith. Thank you guys for your feedback and help!
This PR will allow SensESP lib user configure following features:
In future it will allow any developer of the library to configure more parameters of SensESP from main.cpp. This PR adds more flexibility to SensESP library and defines the way how behaviour for SensESP library is changed by end user.
This PR is backwards compatible with previous code, so no breaking changes are introduced for end user if the end user doesn't change or rely on internals SensESP structures.