mirror of
https://github.com/rstrouse/ESPSomfy-RTS.git
synced 2026-10-31 13:52:15 +01:00
fix: review fixes for step 4a
- e2e: the heap check uses a socket slot now, so it waits for the device to free it, and test_every_client_reconnecting_at_once closes its clients before the check runs. - Comments and docs that still named removed things (the /controller read, the event buffer, memStatus, SocketEmitter, ModelSnapshot). - ESPNetwork::emitSockets is emitStatus: one socket, and it sends the status. - Unused includes, whitespace-only lines left by the toJSON removal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
2a954aa158
commit
cfa298f053
13 changed files with 26 additions and 23 deletions
|
|
@ -84,7 +84,7 @@ The seam through which a shade's behaviour reports what happened: changes for th
|
||||||
|
|
||||||
**Entry point**
|
**Entry point**
|
||||||
Where a request from outside reaches the firmware: an HTTP route, an MQTT command topic or a frame received from a remote.
|
Where a request from outside reaches the firmware: an HTTP route, an MQTT command topic or a frame received from a remote.
|
||||||
_Avoid_: controller (it already names the owner of the shade list and the `/controller` read)
|
_Avoid_: controller (it already names the owner of the shade list)
|
||||||
|
|
||||||
**HttpRoutes**
|
**HttpRoutes**
|
||||||
The HTTP routes on port 80: the web UI's files, login, the file transfers and the three commands for scripts. Everything else is an op on the socket.
|
The HTTP routes on port 80: the web UI's files, login, the file transfers and the three commands for scripts. Everything else is an op on the socket.
|
||||||
|
|
|
||||||
|
|
@ -180,4 +180,4 @@ A malformed message gets a bad request, with its `id` when it had one. A type a
|
||||||
|
|
||||||
## Inside the device
|
## Inside the device
|
||||||
|
|
||||||
async_tcp reads client frames and answers with fixed frames that hold nothing of the shade model: `hello`, errors and `ok`. Everything formatted from the shade model goes out from `loop()`, which is the only task that reads and writes it ([ADR 0002](adr/0002-loop-task-is-the-only-writer-of-the-shade-model.md)). Sync frames and events share the socket event buffer, so protocol 2 needs no more static memory.
|
async_tcp reads client frames and answers with fixed frames that hold nothing of the shade model: `hello`, errors and `ok`. Everything formatted from the shade model goes out from `loop()`, which is the only task that reads and writes it ([ADR 0002](adr/0002-loop-task-is-the-only-writer-of-the-shade-model.md)). Sync frames and events share one frame buffer, which belongs to `loop()`.
|
||||||
|
|
|
||||||
|
|
@ -13,7 +13,7 @@ The shade model (rooms, shades, groups and repeaters) is changed only from `loop
|
||||||
|
|
||||||
## Consequences
|
## Consequences
|
||||||
|
|
||||||
> **Amended by [ADR 0005](0005-one-websocket-protocol.md) (socket protocol 2).** The rule above stands. These consequences are replaced: model edits no longer answer "accepted" and report through `requestResult`, because every socket op gets one reply from `loop()`. Reads no longer go through a ModelSnapshot, because `loop()` formats a sync from the live model. The caps on reads and the UI's 5 s `requestResult` wait go with them.
|
> **Amended by [ADR 0005](0005-one-websocket-protocol.md) (socket protocol 2).** The rule above stands. These consequences are replaced: model edits no longer answer "accepted" and report through `requestResult`, because every socket op gets one reply from `loop()`. Reads no longer go through a ModelSnapshot, because `loop()` formats a sync from the live model. The caps on reads and the UI's 5 s `requestResult` wait go with them, and 4.0.0 removes ModelSnapshot.
|
||||||
|
|
||||||
- async_tcp must still never wait for `loop()`, so a model edit's HTTP reply can't block until the change is made. The reply only says the request was accepted; the result reaches clients as a WebSocket event once `loop()` has made the change. We chose that over a chunked HTTP response that `loop()` completes, because pushing results to clients is what the WebSocket is for and it needs no pool of pending responses.
|
- async_tcp must still never wait for `loop()`, so a model edit's HTTP reply can't block until the change is made. The reply only says the request was accepted; the result reaches clients as a WebSocket event once `loop()` has made the change. We chose that over a chunked HTTP response that `loop()` completes, because pushing results to clients is what the WebSocket is for and it needs no pool of pending responses.
|
||||||
- A read has to reply with data, so it does wait, without blocking: its chunked response sends nothing until `loop()` has filled the read's ModelSnapshot.
|
- A read has to reply with data, so it does wait, without blocking: its chunked response sends nothing until `loop()` has filled the read's ModelSnapshot.
|
||||||
|
|
|
||||||
|
|
@ -5,7 +5,7 @@
|
||||||
#include "esp_log.h"
|
#include "esp_log.h"
|
||||||
#include "ConfigSettings.h"
|
#include "ConfigSettings.h"
|
||||||
#include "ESPNetwork.h"
|
#include "ESPNetwork.h"
|
||||||
#include "entryPoints/Web.h"
|
#include "Somfy.h"
|
||||||
#include "outputs/ProtoSocket.h"
|
#include "outputs/ProtoSocket.h"
|
||||||
#include "Utils.h"
|
#include "Utils.h"
|
||||||
#include "SSDP.h"
|
#include "SSDP.h"
|
||||||
|
|
@ -14,7 +14,6 @@
|
||||||
static const char *TAG = "Network";
|
static const char *TAG = "Network";
|
||||||
|
|
||||||
extern ConfigSettings settings;
|
extern ConfigSettings settings;
|
||||||
extern Web webServer;
|
|
||||||
extern MqttConnection mqtt;
|
extern MqttConnection mqtt;
|
||||||
extern rebootDelay_t rebootDelay;
|
extern rebootDelay_t rebootDelay;
|
||||||
extern ESPNetwork net;
|
extern ESPNetwork net;
|
||||||
|
|
@ -172,7 +171,7 @@ void ESPNetwork::loop() {
|
||||||
}
|
}
|
||||||
if(millis() - this->lastEmit >= STATUS_EMIT_MS) {
|
if(millis() - this->lastEmit >= STATUS_EMIT_MS) {
|
||||||
this->lastEmit = millis();
|
this->lastEmit = millis();
|
||||||
if(this->connected()) this->emitSockets();
|
if(this->connected()) this->emitStatus();
|
||||||
resetTaskWdt(); // Make sure we do not reboot here.
|
resetTaskWdt(); // Make sure we do not reboot here.
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -198,7 +197,7 @@ bool ESPNetwork::changeAP(const uint8_t *bssid, const int32_t channel) {
|
||||||
}
|
}
|
||||||
// The status broadcast, every STATUS_EMIT_MS from loop(). A wired link goes
|
// The status broadcast, every STATUS_EMIT_MS from loop(). A wired link goes
|
||||||
// out every time, the WiFi signal only when it really changed.
|
// out every time, the WiFi signal only when it really changed.
|
||||||
void ESPNetwork::emitSockets() {
|
void ESPNetwork::emitStatus() {
|
||||||
this->emitHeap();
|
this->emitHeap();
|
||||||
#if HAS_ETHERNET
|
#if HAS_ETHERNET
|
||||||
if(this->connType == conn_types_t::ethernet) {
|
if(this->connType == conn_types_t::ethernet) {
|
||||||
|
|
|
||||||
|
|
@ -70,7 +70,7 @@ class ESPNetwork {
|
||||||
bool setup();
|
bool setup();
|
||||||
void loop();
|
void loop();
|
||||||
void end();
|
void end();
|
||||||
void emitSockets();
|
void emitStatus();
|
||||||
void emitHeap();
|
void emitHeap();
|
||||||
uint32_t getChipId();
|
uint32_t getChipId();
|
||||||
static void networkEvent(WiFiEvent_t event);
|
static void networkEvent(WiFiEvent_t event);
|
||||||
|
|
|
||||||
|
|
@ -155,7 +155,7 @@ void setup() {
|
||||||
void loop() {
|
void loop() {
|
||||||
// put your main code here, to run repeatedly:
|
// put your main code here, to run repeatedly:
|
||||||
//uint32_t heap = ESP.getFreeHeap();
|
//uint32_t heap = ESP.getFreeHeap();
|
||||||
// Times the whole pass for memStatus, including any time the loop task was
|
// Times the whole pass for the memory event, including any time the loop task was
|
||||||
// pre-empted, since that delays the loop too.
|
// pre-empted, since that delays the loop too.
|
||||||
ElapsedTimer pass;
|
ElapsedTimer pass;
|
||||||
if(rebootDelay.reboot && millis() > rebootDelay.rebootTime) {
|
if(rebootDelay.reboot && millis() > rebootDelay.rebootTime) {
|
||||||
|
|
|
||||||
|
|
@ -92,7 +92,7 @@ void registerHttpRoutes(AsyncWebServer &server) {
|
||||||
JsonDocument doc;
|
JsonDocument doc;
|
||||||
DeserializationError err = deserializeJson(doc, dataStr);
|
DeserializationError err = deserializeJson(doc, dataStr);
|
||||||
if(err) {
|
if(err) {
|
||||||
request->send(500, "application/json", "{\"status\":\"ERROR\",\"desc\":\"JSON parse error\"}");
|
request->send(500, _encoding_json, "{\"status\":\"ERROR\",\"desc\":\"JSON parse error\"}");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
else {
|
else {
|
||||||
|
|
|
||||||
|
|
@ -10,7 +10,6 @@
|
||||||
#include "Somfy.h"
|
#include "Somfy.h"
|
||||||
#include "WResp.h"
|
#include "WResp.h"
|
||||||
#include "LoopQueue.h"
|
#include "LoopQueue.h"
|
||||||
#include "ItemSnapshot.h"
|
|
||||||
#include "SomfySnapshot.h"
|
#include "SomfySnapshot.h"
|
||||||
#include "Web.h"
|
#include "Web.h"
|
||||||
#include "WebRoutes.h"
|
#include "WebRoutes.h"
|
||||||
|
|
|
||||||
|
|
@ -804,7 +804,7 @@ void ProtoSocket::wifiNetworks(int16_t count) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
bool ProtoSocket::subscribed(proto_feed_t feed) const {
|
bool ProtoSocket::subscribed(proto_feed_t feed) const {
|
||||||
// The feed is formatted into the event buffer, which is loop()'s.
|
// The feed is formatted into the frame buffer, which is loop()'s.
|
||||||
if(!onLoopTask()) return false;
|
if(!onLoopTask()) return false;
|
||||||
const uint8_t bit = 1 << static_cast<uint8_t>(feed);
|
const uint8_t bit = 1 << static_cast<uint8_t>(feed);
|
||||||
for(const proto_slot_t &s : slots)
|
for(const proto_slot_t &s : slots)
|
||||||
|
|
|
||||||
|
|
@ -234,6 +234,9 @@ def heap(host, token=None):
|
||||||
_, frames, _ = run_sync(c, schema)
|
_, frames, _ = run_sync(c, schema)
|
||||||
finally:
|
finally:
|
||||||
c.close()
|
c.close()
|
||||||
|
# The device frees the slot when it sees the disconnect; the caller
|
||||||
|
# may need every slot.
|
||||||
|
time.sleep(0.5)
|
||||||
key = schema.keys["status"]
|
key = schema.keys["status"]
|
||||||
status = schema.expand(next(f["d"][key] for f in frames if key in f["d"]))
|
status = schema.expand(next(f["d"][key] for f in frames if key in f["d"]))
|
||||||
return {"free": status["freeHeap"], "min": status["minHeap"], "max": status["maxHeap"], "uptime": status["uptime"]}
|
return {"free": status["freeHeap"], "min": status["minHeap"], "max": status["maxHeap"], "uptime": status["uptime"]}
|
||||||
|
|
|
||||||
|
|
@ -47,7 +47,9 @@ def all_slots_free(host, token):
|
||||||
pytest.skip("%u socket slot(s) in use, likely by the web UI: close its tabs" % taken)
|
pytest.skip("%u socket slot(s) in use, likely by the web UI: close its tabs" % taken)
|
||||||
|
|
||||||
|
|
||||||
def test_every_client_reconnecting_at_once(connect, schema, watch_heap, all_slots_free):
|
# watch_heap comes before connect so pytest tears connect down first: the
|
||||||
|
# heap check after the test needs a free slot.
|
||||||
|
def test_every_client_reconnecting_at_once(watch_heap, all_slots_free, connect, schema):
|
||||||
clients = [connect() for _ in range(MAX_CLIENTS)]
|
clients = [connect() for _ in range(MAX_CLIENTS)]
|
||||||
errors = []
|
errors = []
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -232,7 +232,7 @@ static void test_emitAllowed_refuses_another_task() {
|
||||||
TEST_ASSERT_FALSE(emitAllowed(&wifiEventTask, &loopTask));
|
TEST_ASSERT_FALSE(emitAllowed(&wifiEventTask, &loopTask));
|
||||||
}
|
}
|
||||||
static void test_emitAllowed_before_the_owner_is_known() {
|
static void test_emitAllowed_before_the_owner_is_known() {
|
||||||
// setup() before SocketEmitter::begin(), which is loop()'s task anyway.
|
// setup() before ProtoSocket::begin(), which is loop()'s task anyway.
|
||||||
TEST_ASSERT_TRUE(emitAllowed(&loopTask, nullptr));
|
TEST_ASSERT_TRUE(emitAllowed(&loopTask, nullptr));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue