fix: review fixes for the OTA slot

- The field is hasOtaSlot, like hasEthernet and the other "has" bools.
- The web UI shows the device's reason for any failed upload, not only
  for /restore, so it shows "no OTA slot" from a page that missed the
  header. The update routes' 500 replies give Update.errorString()
  instead of an empty reason.
- Native: the writer leaves hasOtaSlot out when true and writes "os":0.
- e2e: header() next to uptime(); the test skips on firmware without
  the field instead of failing, and matches the reason loosely. The
  README says what the test sends.
- PROTOCOL.md and the comments name only the builds that exist.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
cjkas 2026-10-28 22:19:27 +01:00
parent 5721d34a34
commit 657b345eaf
13 changed files with 41 additions and 38 deletions

View file

@ -4171,7 +4171,7 @@ class Firmware {
updateFirmware() { updateFirmware() {
let chip = document.getElementById('divContainer').getAttribute('data-chipmodel') || ''; let chip = document.getElementById('divContainer').getAttribute('data-chipmodel') || '';
// Firmware from before the field leaves it out: undefined, not false. // Firmware from before the field leaves it out: undefined, not false.
if (proto.model.header.otaSlot === false) { if (proto.model.header.hasOtaSlot === false) {
ui.infoMessage(`This device has no OTA slot, so its firmware is flashed over USB with SomfyController.onboard.esp32${chip}.bin. That image holds the web UI and an empty configuration too, so take a backup first and restore it afterwards.`); ui.infoMessage(`This device has no OTA slot, so its firmware is flashed over USB with SomfyController.onboard.esp32${chip}.bin. That image holds the web UI and an empty configuration too, so take a backup first and restore it afterwards.`);
return; return;
} }
@ -4307,18 +4307,15 @@ class Firmware {
security.tokenRefused(); security.tokenRefused();
return; return;
} }
if (service !== '/restore' && (xhr.status < 200 || xhr.status >= 300)) { if (xhr.status < 200 || xhr.status >= 300) {
ui.errorMessage(el, `The upload failed (${xhr.status}).`); // The device's reason, when it sent one.
let desc = `The upload failed (${xhr.status}).`;
try { desc = JSON.parse(xhr.responseText).desc || desc; } catch (e) { }
ui.errorMessage(el, desc);
return; return;
} }
switch (service) { switch (service) {
case '/restore': case '/restore':
if (xhr.status < 200 || xhr.status >= 300) {
let desc = 'The restore failed.';
try { desc = JSON.parse(xhr.responseText).desc || desc; } catch (e) { }
ui.errorMessage(el, desc);
break;
}
(async () => { (async () => {
await somfy.init(); await somfy.init();
if (document.getElementById('divUploadFile')) document.getElementById('divUploadFile').remove(); if (document.getElementById('divUploadFile')) document.getElementById('divUploadFile').remove();

View file

@ -140,7 +140,7 @@ ProtoModel.ITEMS = [
// have it. // have it.
ProtoModel.FIELDS = { ProtoModel.FIELDS = {
header: ['maxRooms', 'maxShades', 'maxGroups', 'maxGroupedShades', 'maxLinkedRemotes', 'startingAddress', 'bitLength', header: ['maxRooms', 'maxShades', 'maxGroups', 'maxGroupedShades', 'maxLinkedRemotes', 'startingAddress', 'bitLength',
'proto', 'serverId', 'hostname', 'model', 'chipModel', 'otaSlot', 'fw', 'connType'], 'proto', 'serverId', 'hostname', 'model', 'chipModel', 'hasOtaSlot', 'fw', 'connType'],
room: ['roomId', 'name', 'sortOrder'], room: ['roomId', 'name', 'sortOrder'],
shadeState: ['shadeId', 'position', 'target', 'direction', 'myPos', 'tiltPosition', 'tiltTarget', 'tiltDirection', shadeState: ['shadeId', 'position', 'target', 'direction', 'myPos', 'tiltPosition', 'tiltTarget', 'tiltDirection',
'myTiltPos', 'flags', 'lastRollingCode'], 'myTiltPos', 'flags', 'lastRollingCode'],

View file

@ -53,7 +53,7 @@ A number is never reused. A retired type stays reserved.
- `F`: fields, as `[key, name, kind, default]`, kind being `b` bool, `i` integer, `n` number, `s` string, `l` list, `o` object - `F`: fields, as `[key, name, kind, default]`, kind being `b` bool, `i` integer, `n` number, `s` string, `l` list, `o` object
- `E`: error codes, as `[number, name]` - `E`: error codes, as `[number, name]`
- `Q`: feeds, as `[number, name]` - `Q`: feeds, as `[number, name]`
3. Then the device: `header` (limits, starting address, what a new shade gets, server id, hostname, model, chip, whether it has an OTA slot (`otaSlot`, left out when it has), firmware, connection type), `rooms`, `shades`, `groups`, `repeaters` and `status` (memory, and WiFi or Ethernet). A frame is at most 4 KB, an item is never split across frames, and several sections can share one. 3. Then the device: `header` (limits, starting address, what a new shade gets, server id, hostname, model, chip, whether it has an OTA slot, firmware, connection type), `rooms`, `shades`, `groups`, `repeaters` and `status` (memory, and WiFi or Ethernet). A frame is at most 4 KB, an item is never split across frames, and several sections can share one.
4. The client sends `next` after each `sync` frame, and the device answers with the following one. With no `next` for 30 s the device closes the socket. 4. The client sends `next` after each `sync` frame, and the device answers with the following one. With no `next` for 30 s the device closes the socket.
5. `syncEnd` follows the last `sync` frame. 5. `syncEnd` follows the last `sync` frame.

View file

@ -145,7 +145,7 @@ bool ConfigSettings::begin() {
break; break;
} }
ESP_LOGD(TAG, "Chip Model ESP32-%s", this->chipModel); ESP_LOGD(TAG, "Chip Model ESP32-%s", this->chipModel);
this->otaSlot = esp_ota_get_next_update_partition(nullptr) != nullptr; this->hasOtaSlot = esp_ota_get_next_update_partition(nullptr) != nullptr;
this->fwVersion.parse(FW_VERSION); this->fwVersion.parse(FW_VERSION);
uint64_t mac = ESP.getEfuseMac(); uint64_t mac = ESP.getEfuseMac();
for(int i=0; i<17; i=i+8) { for(int i=0; i<17; i=i+8) {

View file

@ -164,8 +164,8 @@ class ConfigSettings: BaseSettings {
char hostname[32] = "ESPSomfyRTS"; char hostname[32] = "ESPSomfyRTS";
char chipModel[10] = "ESP32"; char chipModel[10] = "ESP32";
// Whether the partition table has a second app slot for /updateFirmware // Whether the partition table has a second app slot for /updateFirmware
// to write. Builds with one 3 MB slot (C5, C6, debug) are flashed over USB. // to write. Builds with one 3 MB slot (C6, debug) are flashed over USB.
bool otaSlot = true; bool hasOtaSlot = true;
conn_types_t connType = conn_types_t::unset; conn_types_t connType = conn_types_t::unset;
appver_t fwVersion; appver_t fwVersion;
appver_t appVersion; appver_t appVersion;

View file

@ -169,7 +169,7 @@
FIELD(hostname, "hn", 's', 0) \ FIELD(hostname, "hn", 's', 0) \
FIELD(model, "md", 's', 0) \ FIELD(model, "md", 's', 0) \
FIELD(chipModel, "cp", 's', 0) \ FIELD(chipModel, "cp", 's', 0) \
FIELD(otaSlot, "os", 'b', 1) \ FIELD(hasOtaSlot, "os", 'b', 1) \
FIELD(connType, "ct", 's', 0) \ FIELD(connType, "ct", 's', 0) \
/* memory, wifi and ethernet */ \ /* memory, wifi and ethernet */ \
FIELD(freeHeap, "fr", 'i', 0) \ FIELD(freeHeap, "fr", 'i', 0) \

View file

@ -165,19 +165,20 @@ void registerHttpRoutes(AsyncWebServer &server) {
server.on(AsyncURIMatcher::exact("/updateFirmware"), HTTP_POST, server.on(AsyncURIMatcher::exact("/updateFirmware"), HTTP_POST,
[](AsyncWebServerRequest *request) { [](AsyncWebServerRequest *request) {
// Nothing was written or stopped, so there is nothing to reboot for. // Nothing was written or stopped, so there is nothing to reboot for.
if(!settings.otaSlot) { if(!settings.hasOtaSlot) {
request->send(400, _encoding_json, F("{\"status\":\"ERROR\",\"desc\":\"This device has no OTA slot, so its firmware is flashed over USB.\"}")); request->send(400, _encoding_json, F("{\"status\":\"ERROR\",\"desc\":\"This device has no OTA slot, so its firmware is flashed over USB.\"}"));
return; return;
} }
// errorString() is one of the updater's fixed messages, none with a quote.
if(Update.hasError()) if(Update.hasError())
request->send(500, _encoding_json, "{\"status\":\"ERROR\",\"desc\":\"Error updating firmware: \"}"); request->send(500, _encoding_json, String("{\"status\":\"ERROR\",\"desc\":\"Error updating firmware: ") + Update.errorString() + "\"}");
else else
request->send(200, _encoding_json, "{\"status\":\"SUCCESS\",\"desc\":\"Successfully updated firmware\"}"); request->send(200, _encoding_json, "{\"status\":\"SUCCESS\",\"desc\":\"Successfully updated firmware\"}");
rebootDelay.reboot = true; rebootDelay.reboot = true;
rebootDelay.rebootTime = millis() + 500; rebootDelay.rebootTime = millis() + 500;
}, },
[](AsyncWebServerRequest *request, String filename, size_t index, uint8_t *data, size_t len, bool final) { [](AsyncWebServerRequest *request, String filename, size_t index, uint8_t *data, size_t len, bool final) {
if(!uploadAllowed(request, index) || !settings.otaSlot) return; if(!uploadAllowed(request, index) || !settings.hasOtaSlot) return;
if(index == 0) { if(index == 0) {
webServer.uploadSuccess = false; webServer.uploadSuccess = false;
ESP_LOGI(TAG, "Update: %s", filename.c_str()); ESP_LOGI(TAG, "Update: %s", filename.c_str());
@ -212,7 +213,7 @@ void registerHttpRoutes(AsyncWebServer &server) {
server.on(AsyncURIMatcher::exact("/updateApplication"), HTTP_POST, server.on(AsyncURIMatcher::exact("/updateApplication"), HTTP_POST,
[](AsyncWebServerRequest *request) { [](AsyncWebServerRequest *request) {
if(Update.hasError()) if(Update.hasError())
request->send(500, _encoding_json, "{\"status\":\"ERROR\",\"desc\":\"Error updating application: \"}"); request->send(500, _encoding_json, String("{\"status\":\"ERROR\",\"desc\":\"Error updating application: ") + Update.errorString() + "\"}");
else else
request->send(200, _encoding_json, "{\"status\":\"SUCCESS\",\"desc\":\"Successfully updated application\"}"); request->send(200, _encoding_json, "{\"status\":\"SUCCESS\",\"desc\":\"Successfully updated application\"}");
rebootDelay.reboot = true; rebootDelay.reboot = true;

View file

@ -77,7 +77,7 @@ static void writeHeader(ProtoWriter &w) {
w.add(proto_field_t::hostname, settings.hostname); w.add(proto_field_t::hostname, settings.hostname);
w.add(proto_field_t::model, "ESPSomfyRTS"); w.add(proto_field_t::model, "ESPSomfyRTS");
w.add(proto_field_t::chipModel, settings.chipModel); w.add(proto_field_t::chipModel, settings.chipModel);
w.add(proto_field_t::otaSlot, settings.otaSlot); w.add(proto_field_t::hasOtaSlot, settings.hasOtaSlot);
w.add(proto_field_t::fw, settings.fwVersion.name); w.add(proto_field_t::fw, settings.fwVersion.name);
w.add(proto_field_t::connType, net.connTypeName()); w.add(proto_field_t::connType, net.connTypeName());
} }

View file

@ -16,7 +16,7 @@ Runs against a real device on your network, by hand, never in CI. It checks sock
| `--lockout` | on a Secured device, also fail `/login` until this PC is refused, then wait out the 30 s lockout | | `--lockout` | on a Secured device, also fail `/login` until this PC is refused, then wait out the 30 s lockout |
| `--record` | save hello, the sync and the schema to `test/fixtures/proto2/`, for the web UI and Home Assistant tests | | `--record` | save hello, the sync and the schema to `test/fixtures/proto2/`, for the web UI and Home Assistant tests |
A plain run sends nothing on air, changes no setting (it saves only values the device already has) and doesn't reboot the device. The command tests send ops that are refused before they reach the radio, and one `setPositions` that sets a shade to the position the sync gave: that drops the fraction of a percent the shade's believed position may have had. The edit tests send edits that are refused, and one shade sort order set to the order it has, which saves the shades again unchanged. The settings tests send saves that are refused, and one that saves the NTP server and time zone the device already has. The restore test sends options that aren't JSON with a file that isn't a backup, so nothing is restored. The frequency scan test pauses the radio's sending for a second or two, as a scan does. It does fill every socket slot for a few seconds, and replays a page load. A plain run sends nothing on air, changes no setting (it saves only values the device already has) and doesn't reboot the device. The command tests send ops that are refused before they reach the radio, and one `setPositions` that sets a shade to the position the sync gave: that drops the fraction of a percent the shade's believed position may have had. The edit tests send edits that are refused, and one shade sort order set to the order it has, which saves the shades again unchanged. The settings tests send saves that are refused, and one that saves the NTP server and time zone the device already has. The restore test sends options that aren't JSON with a file that isn't a backup, so nothing is restored. A firmware upload goes only to a device without an OTA slot, which refuses it. The frequency scan test pauses the radio's sending for a second or two, as a scan does. It does fill every socket slot for a few seconds, and replays a page load.
The web UI holds one of the device's 5 socket slots for each open tab, and Home Assistant holds one. The tests that fill every slot fill the ones left, so close the UI's tabs for the full load. The web UI holds one of the device's 5 socket slots for each open tab, and Home Assistant holds one. The tests that fill every slot fill the ones left, so close the UI's tabs for the full load.

View file

@ -12,7 +12,7 @@ import urllib.request
import pytest import pytest
from proto_client import ProtoClient, run_sync from proto_client import ProtoClient, run_sync
from test_settings import uptime from test_settings import header, uptime
# Every route that needs the token under Secured, sent without a body or # Every route that needs the token under Secured, sent without a body or
# parameters, so a route that let one through would still change nothing. # parameters, so a route that let one through would still change nothing.
@ -173,18 +173,16 @@ def test_a_socket_from_the_page_the_device_served_is_accepted(host, token):
def test_a_firmware_upload_without_an_ota_slot_is_refused(host, token, connect, schema): def test_a_firmware_upload_without_an_ota_slot_is_refused(host, token, connect, schema):
"""Builds with one app slot (C5, C6, debug) say so in the header, and """Builds with one app slot (C6, debug) say so in the header, and refuse
refuse the upload without stopping the radio or rebooting.""" the upload without stopping the radio or rebooting."""
c = connect(schema=schema.hash) _, frames, _ = run_sync(connect(schema=schema.hash))
_, frames, _ = run_sync(c) # Left out at its default, true, as is a field older firmware doesn't have.
key = schema.keys["header"] if header(schema, frames).get("hasOtaSlot", True):
header = next(schema.expand(f["d"][key]) for f in frames if key in f["d"])
if header.get("otaSlot", schema.default("otaSlot")):
pytest.skip("the device has an OTA slot: an upload would start an update") pytest.skip("the device has an OTA slot: an upload would start an update")
uptime_before = uptime(schema, frames) uptime_before = uptime(schema, frames)
status, _, body = upload(host, "/updateFirmware", auth(token)) status, _, body = upload(host, "/updateFirmware", auth(token))
assert status == 400, body assert status == 400, body
assert json.loads(body)["desc"] == "This device has no OTA slot, so its firmware is flashed over USB." assert "no OTA slot" in json.loads(body)["desc"]
# The old reply rebooted half a second later. # The old reply rebooted half a second later.
time.sleep(2) time.sleep(2)
_, frames, _ = run_sync(connect(schema=schema.hash)) _, frames, _ = run_sync(connect(schema=schema.hash))

View file

@ -86,9 +86,8 @@ def test_a_save_tells_the_other_clients(connect, schema):
def test_a_wifi_scan_lists_the_networks(connect, schema): def test_a_wifi_scan_lists_the_networks(connect, schema):
c = connect(schema=schema.hash) c = connect(schema=schema.hash)
_, frames, _ = run_sync(c) _, frames, _ = run_sync(c)
header = next(schema.expand(f["d"][schema.keys["header"]]) for f in frames if schema.keys["header"] in f["d"])
# The connection in use: Ethernet with WiFi as the fallback may be on either. # The connection in use: Ethernet with WiFi as the fallback may be on either.
if header.get("connType") != "Wifi": if header(schema, frames).get("connType") != "Wifi":
pytest.skip("the device isn't on WiFi") pytest.skip("the device isn't on WiFi")
assert is_ok(schema, op(c, schema, "scanWifi", {})) assert is_ok(schema, op(c, schema, "scanWifi", {}))
found = c.recv_until(lambda f: f["t"] == schema.type_numbers["wifiNetworks"], timeout=20)[-1] found = c.recv_until(lambda f: f["t"] == schema.type_numbers["wifiNetworks"], timeout=20)[-1]
@ -152,6 +151,12 @@ def wait_for_restart(host, token, schema, uptime_before):
time.sleep(2) time.sleep(2)
def header(schema, frames):
"""The header a sync starts with."""
key = schema.keys["header"]
return next(schema.expand(f["d"][key]) for f in frames if key in f["d"])
def uptime(schema, frames): def uptime(schema, frames):
"""The uptime, in ms, from the status a sync ends with.""" """The uptime, in ms, from the status a sync ends with."""
key = schema.keys["status"] key = schema.keys["status"]

View file

@ -37,17 +37,17 @@ test('a field the sync left out is its default', () => {
for (const g of m.groupList()) assert.ok(Array.isArray(g.linkedShades)); for (const g of m.groupList()) assert.ok(Array.isArray(g.linkedShades));
}); });
test('otaSlot is false only when the device says so', () => { test('the device has an OTA slot unless it says it has none', () => {
// The recording is from firmware before the field. // The recording is from firmware before the field.
assert.equal(synced().header.otaSlot, undefined); assert.equal(synced().header.hasOtaSlot, undefined);
const schema = recordedSchema(); const schema = recordedSchema();
schema.add({ F: [['os', 'otaSlot', 'b', 1]] }); schema.add({ F: [['os', 'hasOtaSlot', 'b', 1]] });
const header = deviceSections(schema).find((d) => d.header).header; const header = deviceSections(schema).find((d) => d.header).header;
const m = new ProtoModel(schema); const m = new ProtoModel(schema);
m.applySync({ header: header }); m.applySync({ header: header });
assert.equal(m.header.otaSlot, true); assert.equal(m.header.hasOtaSlot, true);
m.applySync({ header: schema.expand(Object.assign({ os: 0 }, schema.pack(header))) }); m.applySync({ header: schema.expand(Object.assign({ os: 0 }, schema.pack(header))) });
assert.equal(m.header.otaSlot, false); assert.equal(m.header.hasOtaSlot, false);
}); });
test('the lists come in their sort order', () => { test('the lists come in their sort order', () => {

View file

@ -119,15 +119,17 @@ static void test_values_at_their_default_are_left_out() {
w.add(proto_field_t::myPos, (int8_t)-1); w.add(proto_field_t::myPos, (int8_t)-1);
w.add(proto_field_t::repeats, (uint8_t)1); w.add(proto_field_t::repeats, (uint8_t)1);
w.add(proto_field_t::paired, false); w.add(proto_field_t::paired, false);
w.add(proto_field_t::hasOtaSlot, true); // a bool whose default is true
w.add(proto_field_t::name, ""); w.add(proto_field_t::name, "");
w.add(proto_field_t::name, (const char *)nullptr); w.add(proto_field_t::name, (const char *)nullptr);
}).c_str()); }).c_str());
} }
static void test_values_off_their_default_are_written_by_key() { static void test_values_off_their_default_are_written_by_key() {
TEST_ASSERT_EQUAL_STRING("{\"p\":50,\"m\":0,\"i\":1,\"n\":\"Office\",\"a\":4294967295,\"q\":-5}", writeObject([](ProtoWriter &w) { TEST_ASSERT_EQUAL_STRING("{\"p\":50,\"m\":0,\"i\":1,\"os\":0,\"n\":\"Office\",\"a\":4294967295,\"q\":-5}", writeObject([](ProtoWriter &w) {
w.add(proto_field_t::position, (int8_t)50); w.add(proto_field_t::position, (int8_t)50);
w.add(proto_field_t::myPos, (int8_t)0); w.add(proto_field_t::myPos, (int8_t)0);
w.add(proto_field_t::paired, true); w.add(proto_field_t::paired, true);
w.add(proto_field_t::hasOtaSlot, false);
w.add(proto_field_t::name, "Office"); w.add(proto_field_t::name, "Office");
w.add(proto_field_t::remoteAddress, (uint32_t)0xFFFFFFFF); w.add(proto_field_t::remoteAddress, (uint32_t)0xFFFFFFFF);
w.add(proto_field_t::tiltPosition, (int8_t)-5); w.add(proto_field_t::tiltPosition, (int8_t)-5);