Esp form uiupdate #36
Conversation
.vscode/settings.json
Outdated
| @@ -8,6 +8,7 @@ | |||
| "vector": "cpp", | |||
| "string_view": "cpp", | |||
| "initializer_list": "cpp", | |||
| "cmath": "cpp" | |||
| "cmath": "cpp", | |||
| "*.tcc": "cpp" | |||
There was a problem hiding this comment.
Why do we need this line?
| strlcpy(_detector_list[i].metadata, metadataStr.c_str(), sizeof(_detector_list[i].metadata)); | ||
| } else { | ||
| _detector_list[i].metadata[0] = '\0'; | ||
| } |
There was a problem hiding this comment.
An awesome bonus here would be to check the length of the string before you copy it into the char array. We've had memory overflow problems before on, for example, detectors with ridiculously long queries.
There was a problem hiding this comment.
I thought if I use strlcpy to copy the string, it specifies the size of string so that overflow won't happen?
There was a problem hiding this comment.
Admittedly not strictly a part of your PR, but just above here are some unsafe strcpy calls
| @@ -52,7 +52,8 @@ build_flags = | |||
| ${env.build_flags} | |||
| '-D CAMERA_MODEL_M5STACK_PSRAM' | |||
| '-D NAME="GROUNDLIGHT_DEMO_UNIT"' | |||
| # '-D ENABLE_AP' | |||
| '-D ENABLE_AP' | |||
| # '-D ENABLE_STACKLIGHT' | |||
There was a problem hiding this comment.
Do the demo units no longer work with Stacklight?
There was a problem hiding this comment.
Yes we are skipping stacklight connection for now due to connection issues on my end, and once I confirmed the stacklight is functioning, I will enable it again.
src/deployable_example.cpp
Outdated
| @@ -170,7 +170,7 @@ int start_hr = 8; | |||
| int end_hr = 17; | |||
|
|
|||
| bool disable_deep_sleep_for_notifications = false; | |||
| bool disable_deep_sleep_until_reset = false; | |||
| bool disable_deep_sleep_until_reset = true; | |||
There was a problem hiding this comment.
Why do we want to disable deep sleep on the first loop?
There was a problem hiding this comment.
So that the access point remains active, and we can still access the local web page to update configuration settings later on.
| Twilio SID: <input type="text" name="twilio_sid" value="%twilio_sid%"> | ||
| Twilio Token: <input type="text" name="twilio_token" value="%twilio_token%"> | ||
| Twilio Number: <input type="text" name="twilio_number" value="%twilio_number%"> | ||
| Twilio Recipient: <input type="text" name="twilio_recipient" value="%twilio_recipient%"> |
There was a problem hiding this comment.
Are we intentionally killing support for notifications off the camera?
There was a problem hiding this comment.
Yes I killed the support for notifications because it is currently supported through the web app.
src/deployable_example.cpp
Outdated
| @@ -787,6 +838,8 @@ void loop () { | |||
| } | |||
| if (WiFi.isConnected()) { | |||
| debug_printf("WIFI connected to SSID %s\n", ssid); | |||
|
|
|||
There was a problem hiding this comment.
We should look into adding a linter for these spare lines
| for (int i = 0; i < detectors.size; i++) { | ||
| if (String(detectors.detectors[i].id) == String(detectorId)) { | ||
| return detectors.detectors[i]; | ||
| det = detectors.detectors[i]; |
There was a problem hiding this comment.
can probably stop iterating ?
src/deployable_example.cpp
Outdated
| Motion Alpha (float between 0 and 1): <input type="text" name="mot_a" value="%mot_a%"> | ||
| Motion Beta (float between 0 and 1): <input type="text" name="mot_b" value="%mot_b%"> | ||
| </div> | ||
| Enable Stacklight: <input type="checkbox" id="stacklightCheckbox" name="stacklightbox" value="true" onchange="toggleStacklightSettings()"> |
There was a problem hiding this comment.
if stacklight is currently disabled, should this be disabled as well ?
src/deployable_example.cpp
Outdated
| detector esp_det = get_detector_by_name(endpoint, esp_detector_name.c_str(), apiToken); | ||
| if (strcmp(esp_det.id, "NONE") == 0) { | ||
| preferences.putString("det_id", "DETECTOR NOT FOUND"); | ||
| Serial.println("Error: Detector not found."); |
There was a problem hiding this comment.
should probably be consistent about using the debug_printf vs Serial.println ?
Implemented autoconfig feature for the localhost web page.