Merge pull request #100 from cjkas/feat/proto-command-ops

feat: command ops for socket protocol 2

docs/PROTOCOL.md

@@ -39,8 +39,8 @@ The keys of `hello` (`fw`, `sh`) and of the schema itself (`T`, `F`, `E`, `Q`) n
 | Range | Sender | Types |
 |---|---|---|
 | 1–15 | control | `hello` 1, `reply` 2, `next` 3, `sync` 4, `syncEnd` 5, `subscribe` 6, `unsubscribe` 7 |
-| 16–127 | client ops | none yet; commands, model edits and settings follow |
+| 16–127 | client ops | commands: `shadeCommand` 16, `groupCommand` 17, `tiltCommand` 18, `repeatCommand` 19, `setPositions` 20, `setSensor` 21, `setMyPosition` 22, `unpairShade` 23, `setRollingCode` 24, `sendRemoteCommand` 25; model edits and settings follow |
-| 128–255 | device events | `shadeState`, `shade`, `shadeRemoved`, `group`, `groupState`, `groupRemoved`, `room`, `roomRemoved`, `roomOrder`, `shadeOrder`, `groupOrder`, `shadeCommand`, `memory`, `wifi`, `ethernet`, `frame`, `frequencyScan`, `repeaters` |
+| 128–255 | device events | `shadeState`, `shade`, `shadeRemoved`, `group`, `groupState`, `groupRemoved`, `room`, `roomRemoved`, `roomOrder`, `shadeOrder`, `groupOrder`, `shadeCommanded`, `memory`, `wifi`, `ethernet`, `frame`, `frequencyScan`, `repeaters` |
 
 A number is never reused. A retired type stays reserved.
 
@@ -71,9 +71,37 @@ Events go out to every client from the moment it connects, so they can arrive du
 | `*Removed` | a shade, group or room was deleted | its id |
 | `roomOrder`, `shadeOrder`, `groupOrder` | the sort order changed | `is`: the ids in order |
 | `repeaters` | a repeater was linked or unlinked | the list |
-| `shadeCommand` | a remote or the device commanded a shade | id, address, command, source, rolling code, source address |
+| `shadeCommanded` | a remote or the device commanded a shade | id, address, command, source, rolling code, source address |
 | `memory`, `wifi`, `ethernet` | every 30 s, WiFi only when the signal moved | |
 
+## Commands
+
+Every op carries an `id` and gets one reply. The ops do what the HTTP routes of the same name do, with these arguments:
+
+| Op | Payload | Does |
+|---|---|---|
+| `shadeCommand` | `shadeId`; `command` or `target`; optional `repeat`, `stepSize` | sends the command, or moves the shade to the position 0–100 |
+| `tiltCommand` | `shadeId`; `command` or `target` | the same for the tilt |
+| `groupCommand` | `groupId`, `command`; optional `repeat`, `stepSize` | sends the command as the group |
+| `repeatCommand` | `shadeId` or `groupId`, `command`; optional `repeat`, `stepSize` | repeats the last frame when it was this command, or sends it |
+| `setPositions` | `shadeId`; `position`, `tiltPosition` or both | sets where the shade is thought to be; nothing goes on air |
+| `setSensor` | `shadeId` or `groupId`; `sunny`, `windy` or both; optional `repeat` | sends a sensor frame |
+| `setMyPosition` | `shadeId`; `myPos`, `myTiltPos` or both | sets the favorite |
+| `unpairShade` | `shadeId` | sends Prog and clears paired |
+| `setRollingCode` | `shadeId`, `lastRollingCode` | sets the next rolling code; nothing goes on air |
+| `sendRemoteCommand` | `remoteAddress`, `command`, `lastRollingCode`; optional `encKey`, `repeats` | sends one frame as a remote that isn't one of the shades |
+
+- **An op reads only the fields that were sent.** Unlike what the device sends, a field left out of an op is not its schema default: it is not given, so `setPositions` with only `position` leaves the tilt as it is. A client sends every field it means, defaults included.
+- `command` is a command's name, in any case: `My`, `Up`, `Down`, `MyUp`, `MyDown`, `UpDown`, `MyUpDown`, `Prog`, `SunFlag`, `Flag`, `StepUp`, `StepDown`, `Toggle`, `Sensor`, `Favorite`, `Stop`, or the spelling `shadeCommanded` reports (`Step Up`, `My+Up`).
+- `repeat` left out means the shade's or the group's own repeats.
+- Where they differ from the routes:
+  - The keys come from the dictionary: `setMyPosition` takes `myPos` and `myTiltPos` (the route: `pos`, `tilt`); `setRollingCode` and `sendRemoteCommand` take `lastRollingCode` (the routes: `rollingCode`, `rcode`); `sendRemoteCommand` takes `remoteAddress` (the route: `address`).
+  - `groupCommand`, `repeatCommand` and `sendRemoteCommand` need a `command`, where the routes send My without one.
+  - `setMyPosition` takes a tilt on its own and keeps the lift favorite, where the route needs a lift position.
+- A field of the wrong kind, a value out of range, or both of two alternatives, is a bad request, and `desc` names the field.
+- The reply comes once the command has gone to the radio, or was refused: not found (no shade or group with that id), radio unavailable (the radio is paused, as during a frequency scan, or didn't take it), busy (the device's command queue is full, or `setMyPosition` on a shade that is moving) or bad request (`setPositions` with only a tilt, for a shade without one). It carries no state: the shade's `shadeState` events follow as it moves.
+- `sendRemoteCommand` is answered as soon as the radio has taken the frame, not after the ops queued before it, and with radio unavailable when the radio is full rather than waiting for room.
+
 ## Replies
 
 A client message with an `id` gets exactly one `reply` with that `id`, to that client only, whether it is an op or a control message such as `next`: `ok` 1 on success, or an error `code` and a `desc` for people. Error codes are in the schema: bad request, unknown op, unauthorized, not found, busy, radio unavailable, internal.

platformio.ini

@@ -108,7 +108,7 @@ build_flags =
 platform = native
 test_framework = unity
 test_build_src = yes
-build_src_filter = -<*> +<Utils.cpp> +<SomfyFrame.cpp> +<JsonFormatter.cpp> +<ShadeMotion.cpp> +<ShadeSensors.cpp> +<ShadeBehaviour.cpp> +<PulseDecoder.cpp> +<TxBuffer.cpp> +<RadioScheduler.cpp> +<ShadeRequests.cpp> +<entryPoints/MqttCommands.cpp> +<ModelSnapshot.cpp> +<ModelEditor.cpp> +<ShadeChanges.cpp> +<ProtoSchema.cpp> +<ProtoFrame.cpp> +<ProtoSync.cpp> +<ProtoParse.cpp> +<ProtoSlots.cpp>
+build_src_filter = -<*> +<Utils.cpp> +<SomfyFrame.cpp> +<JsonFormatter.cpp> +<ShadeMotion.cpp> +<ShadeSensors.cpp> +<ShadeBehaviour.cpp> +<PulseDecoder.cpp> +<TxBuffer.cpp> +<RadioScheduler.cpp> +<ShadeRequests.cpp> +<entryPoints/MqttCommands.cpp> +<ModelSnapshot.cpp> +<ModelEditor.cpp> +<ShadeChanges.cpp> +<ProtoSchema.cpp> +<ProtoFrame.cpp> +<ProtoSync.cpp> +<ProtoParse.cpp> +<ProtoSlots.cpp> +<ProtoOps.cpp>
 lib_deps =
 	bblanchon/ArduinoJson@^7.2.2
 build_flags =

src/LoopQueue.cpp

@@ -8,6 +8,7 @@
 #include "SomfySnapshot.h"
 #include "SomfyEditor.h"
 #include "Utils.h"
+#include "outputs/ProtoSocket.h"
 
 static const char *TAG = "LoopQueue";
 
@@ -50,10 +51,13 @@ void LoopQueue::drain() {
     // loop() is the only consumer so this is the item we peeked.
     xQueueReceive(q, &item, 0);
     switch(item.type) {
-      case loop_item_type_t::shade_request:
+      case loop_item_type_t::shade_request: {
-        if(!shadeRequests.run(item.request))
+        const shade_request_result_t result = shadeRequests.run(item.request);
+        if(result == shade_request_result_t::radio_refused)
           ESP_LOGW(TAG, "Radio didn't take shade request %u", static_cast<uint8_t>(item.request.type));
+        if(item.replyClient) protoSocket.replyRequest(item.replyClient, item.replyId, item.request, result);
         break;
+      }
       case loop_item_type_t::restore_shades: {
         restore_options_t opts = restoreOptions(item.restoreParts);
         const bool restored = ShadeConfigFile::restore(&somfy, "/shades.tmp", opts);

src/LoopQueue.h

@@ -33,17 +33,24 @@ struct loop_item_t {
   uint8_t restoreParts = 0;       // restore_shades: RESTORE_* bits
   uint8_t slot = 0;               // snapshot_fill: the SnapshotSlots slot
   model_edit_t *edit = nullptr;   // model_edit: loop() runs and frees it
+  // shade_request from a protocol 2 client: loop() replies to this socket
+  // client with replyId once the request has run.  0 for no reply.
+  uint32_t replyClient = 0;
+  uint32_t replyId = 0;
 };
 
 // Whether the item sends anything on air.
 inline bool loopItemUsesRadio(const loop_item_t &item) {
   return item.type == loop_item_type_t::shade_request && shadeRequestUsesRadio(item.request.type);
 }
-// Whether the item at the head of the queue waits for the radio.  The client
+// Whether the item at the head of the queue waits for the radio.  An HTTP or
-// was already told it was accepted so it stays queued until the radio has room
+// MQTT client was already told it was accepted, so it stays queued until the
-// rather than have the radio drop it.  While TX is suspended nothing gets
+// radio has room rather than have the radio drop it; a protocol 2 client gets
-// through anyway so nothing is held back then, except an unpair: the client was
+// its reply once it has run.  While TX is suspended nothing gets through
-// told the shade is unpaired, so it waits until TX resumes.
+// anyway so nothing is held back then, except an unpair: an HTTP client was
+// told the shade is unpaired, so it waits until TX resumes.  Everything queued
+// behind it waits too, protocol 2 replies included, so a frequency scan
+// started after an unpair delays them until it ends.
 inline bool holdLoopItem(const loop_item_t &item, bool txSuspended, bool txHasRoom) {
   if(txSuspended) return item.type == loop_item_type_t::shade_request && item.request.type == shade_request_type_t::unpair;
   return loopItemUsesRadio(item) && !txHasRoom;

src/ProtoOps.cpp

@@ -0,0 +1,244 @@
+#include <stdio.h>
+#include <strings.h>
+#include "ProtoOps.h"
+
+// An op reads only the fields that were sent: unlike what the device sends, a
+// left-out field is not its schema default but not given, so a client can
+// leave a shade's tilt or its sun flag as they are.
+class OpReader {
+  public:
+    explicit OpReader(JsonObjectConst d) : d(d) {}
+    bool has(proto_field_t field) const { return !this->d[protoField(field).key].isNull(); }
+    // The field when it was sent, as an integer in lo..hi.  False when it
+    // wasn't; error is set when it was but isn't one.
+    bool get(proto_field_t field, int32_t lo, int32_t hi, int32_t &out) {
+      JsonVariantConst v = this->d[protoField(field).key];
+      if(v.isNull()) return false;
+      if(!v.is<int32_t>() || v.as<int32_t>() < lo || v.as<int32_t>() > hi) {
+        this->fail("%s must be %ld-%ld", protoField(field).name, (long)lo, (long)hi);
+        return false;
+      }
+      out = v.as<int32_t>();
+      return true;
+    }
+    // The same for a field the op can't do without.
+    bool need(proto_field_t field, int32_t lo, int32_t hi, int32_t &out) {
+      if(!this->has(field)) {
+        this->fail("%s is missing", protoField(field).name);
+        return false;
+      }
+      return this->get(field, lo, hi, out);
+    }
+    bool command(somfy_commands &cmd) {
+      if(!this->has(proto_field_t::command)) {
+        this->fail("command is missing");
+        return false;
+      }
+      if(!protoCommand(protoString(proto_field_t::command), cmd)) {
+        this->fail("Not a command");
+        return false;
+      }
+      return true;
+    }
+    // Exactly one of the two.
+    bool either(proto_field_t a, proto_field_t b) {
+      if(this->has(a) == this->has(b)) {
+        this->fail("Give %s or %s", protoField(a).name, protoField(b).name);
+        return false;
+      }
+      return true;
+    }
+    template<typename... Args>
+    void fail(const char *fmt, Args... args) {
+      // Only the first problem is reported.
+      if(this->error) return;
+      snprintf(this->why, sizeof(this->why), fmt, args...);
+      this->error = this->why;
+    }
+    const char *error = nullptr;
+  private:
+    JsonObjectConst d;
+    char why[64];
+    const char *protoString(proto_field_t field) const {
+      JsonVariantConst v = this->d[protoField(field).key];
+      return v.is<const char *>() ? v.as<const char *>() : nullptr;
+    }
+};
+// The reader's problem, kept once the reader has gone so the reply can send
+// it.  One buffer for every parse: they all run on async_tcp, one at a time.
+static char lastError[64];
+static const char *errorOf(OpReader &r) {
+  if(!r.error) return nullptr;
+  snprintf(lastError, sizeof(lastError), "%s", r.error);
+  return lastError;
+}
+
+struct command_name_t {
+  const char *name;
+  somfy_commands cmd;
+};
+// The names, and the spellings shadeCommanded reports, so they read back.
+static const command_name_t COMMANDS[] = {
+  {"My", somfy_commands::My}, {"Up", somfy_commands::Up}, {"Down", somfy_commands::Down},
+  {"MyUp", somfy_commands::MyUp}, {"My+Up", somfy_commands::MyUp},
+  {"MyDown", somfy_commands::MyDown}, {"My+Down", somfy_commands::MyDown},
+  {"UpDown", somfy_commands::UpDown}, {"Up+Down", somfy_commands::UpDown},
+  {"MyUpDown", somfy_commands::MyUpDown}, {"My+Up+Down", somfy_commands::MyUpDown},
+  {"Prog", somfy_commands::Prog},
+  {"SunFlag", somfy_commands::SunFlag}, {"Sun Flag", somfy_commands::SunFlag},
+  {"Flag", somfy_commands::Flag},
+  {"StepUp", somfy_commands::StepUp}, {"Step Up", somfy_commands::StepUp},
+  {"StepDown", somfy_commands::StepDown}, {"Step Down", somfy_commands::StepDown},
+  {"Toggle", somfy_commands::Toggle}, {"Sensor", somfy_commands::Sensor},
+  {"Favorite", somfy_commands::Favorite}, {"Stop", somfy_commands::Stop}
+};
+bool protoCommand(const char *name, somfy_commands &cmd) {
+  if(!name) return false;
+  for(const command_name_t &c : COMMANDS) {
+    if(strcasecmp(c.name, name) == 0) {
+      cmd = c.cmd;
+      return true;
+    }
+  }
+  return false;
+}
+
+bool protoIsShadeOp(uint8_t type) {
+  return type >= static_cast<uint8_t>(proto_type_t::shadeCommand) && type <= static_cast<uint8_t>(proto_type_t::setRollingCode);
+}
+
+// The repeat count and step size a command may carry.
+static void readRepeat(OpReader &r, shade_request_t &req) {
+  int32_t n;
+  if(r.get(proto_field_t::repeat, 0, INT8_MAX, n)) req.repeat = static_cast<int8_t>(n);
+}
+static void readStep(OpReader &r, shade_request_t &req) {
+  int32_t n;
+  // Steps are 7 bits on air.
+  if(r.get(proto_field_t::stepSize, 0, 127, n)) req.stepSize = static_cast<uint8_t>(n);
+}
+static void readId(OpReader &r, proto_field_t field, shade_request_t &req) {
+  int32_t n;
+  if(r.need(field, 0, 255, n)) req.id = static_cast<uint8_t>(n);
+}
+static void readShade(OpReader &r, shade_request_t &req) { readId(r, proto_field_t::shadeId, req); }
+// A shade or a group; req.type becomes the one that was given.
+static void readShadeOrGroup(OpReader &r, shade_request_t &req, shade_request_type_t shade, shade_request_type_t group) {
+  if(!r.either(proto_field_t::shadeId, proto_field_t::groupId)) return;
+  const bool isShade = r.has(proto_field_t::shadeId);
+  req.type = isShade ? shade : group;
+  readId(r, isShade ? proto_field_t::shadeId : proto_field_t::groupId, req);
+}
+// A command, or a position 0-100 to move to.
+static void readCommandOrTarget(OpReader &r, shade_request_t &req) {
+  if(!r.either(proto_field_t::command, proto_field_t::target)) return;
+  int32_t n;
+  if(r.has(proto_field_t::command)) r.command(req.command);
+  else if(r.get(proto_field_t::target, 0, 100, n)) req.target = static_cast<int8_t>(n);
+}
+// Two values in lo..hi of which at least one is given; the other is left as
+// it was.
+static void readOneOrBoth(OpReader &r, proto_field_t a, proto_field_t b, int32_t lo, int32_t hi, int8_t &aOut, int8_t &bOut) {
+  if(!r.has(a) && !r.has(b)) {
+    r.fail("Give %s, %s or both", protoField(a).name, protoField(b).name);
+    return;
+  }
+  int32_t n;
+  if(r.get(a, lo, hi, n)) aOut = static_cast<int8_t>(n);
+  if(r.get(b, lo, hi, n)) bOut = static_cast<int8_t>(n);
+}
+
+const char *parseShadeOp(uint8_t type, JsonObjectConst d, shade_request_t &req) {
+  req = shade_request_t();
+  OpReader r(d);
+  int32_t n;
+  switch(static_cast<proto_type_t>(type)) {
+    case proto_type_t::shadeCommand:
+      req.type = shade_request_type_t::shade_command;
+      readShade(r, req);
+      readCommandOrTarget(r, req);
+      readRepeat(r, req);
+      readStep(r, req);
+      break;
+    case proto_type_t::tiltCommand:
+      req.type = shade_request_type_t::tilt_command;
+      readShade(r, req);
+      readCommandOrTarget(r, req);
+      break;
+    case proto_type_t::groupCommand:
+      req.type = shade_request_type_t::group_command;
+      readId(r, proto_field_t::groupId, req);
+      r.command(req.command);
+      readRepeat(r, req);
+      readStep(r, req);
+      break;
+    case proto_type_t::repeatCommand:
+      readShadeOrGroup(r, req, shade_request_type_t::shade_repeat, shade_request_type_t::group_repeat);
+      r.command(req.command);
+      readRepeat(r, req);
+      readStep(r, req);
+      break;
+    case proto_type_t::setPositions:
+      req.type = shade_request_type_t::set_positions;
+      readShade(r, req);
+      readOneOrBoth(r, proto_field_t::position, proto_field_t::tiltPosition, 0, 100, req.position, req.tiltPosition);
+      break;
+    case proto_type_t::setSensor:
+      readShadeOrGroup(r, req, shade_request_type_t::shade_sensor, shade_request_type_t::group_sensor);
+      readOneOrBoth(r, proto_field_t::sunny, proto_field_t::windy, 0, 1, req.sunny, req.windy);
+      readRepeat(r, req);
+      break;
+    case proto_type_t::setMyPosition:
+      req.type = shade_request_type_t::set_favorite;
+      readShade(r, req);
+      readOneOrBoth(r, proto_field_t::myPos, proto_field_t::myTiltPos, 0, 100, req.position, req.tiltPosition);
+      break;
+    case proto_type_t::unpairShade:
+      req.type = shade_request_type_t::unpair;
+      readShade(r, req);
+      break;
+    case proto_type_t::setRollingCode:
+      req.type = shade_request_type_t::set_rolling_code;
+      readShade(r, req);
+      if(r.need(proto_field_t::lastRollingCode, 0, UINT16_MAX, n)) req.rollingCode = static_cast<uint16_t>(n);
+      break;
+    default:
+      r.fail("Not a shade op");
+      break;
+  }
+  return errorOf(r);
+}
+
+const char *parseRemoteCommandOp(JsonObjectConst d, somfy_frame_t &frame, uint8_t &repeats) {
+  frame = somfy_frame_t();
+  repeats = 0;
+  OpReader r(d);
+  int32_t n;
+  // A remote address is 24 bits, and 0 is no remote.
+  if(r.need(proto_field_t::remoteAddress, 1, 0xFFFFFF, n)) frame.remoteAddress = static_cast<uint32_t>(n);
+  r.command(frame.cmd);
+  if(r.need(proto_field_t::lastRollingCode, 1, UINT16_MAX, n)) frame.rollingCode = static_cast<uint16_t>(n);
+  if(r.get(proto_field_t::encKey, 0, UINT8_MAX, n)) frame.encKey = static_cast<uint8_t>(n);
+  if(r.get(proto_field_t::repeats, 0, UINT8_MAX, n)) repeats = static_cast<uint8_t>(n);
+  return errorOf(r);
+}
+
+proto_error_t protoRequestError(shade_request_result_t result) {
+  switch(result) {
+    case shade_request_result_t::not_found: return proto_error_t::notFound;
+    case shade_request_result_t::radio_refused: return proto_error_t::radioUnavailable;
+    // At rest it would take it: the client may try again.
+    case shade_request_result_t::shade_moving: return proto_error_t::busy;
+    case shade_request_result_t::no_tilt: return proto_error_t::badRequest;
+    default: return proto_error_t::none;
+  }
+}
+const char *protoRequestErrorDesc(const shade_request_t &req, shade_request_result_t result) {
+  switch(result) {
+    case shade_request_result_t::not_found: return shadeRequestIsForGroup(req.type) ? "No group with that id" : "No shade with that id";
+    case shade_request_result_t::radio_refused: return PROTO_RADIO_REFUSED;
+    case shade_request_result_t::shade_moving: return "The shade is moving";
+    case shade_request_result_t::no_tilt: return "The shade has no tilt";
+    default: return nullptr;
+  }
+}

src/ProtoOps.h

@@ -0,0 +1,37 @@
+#ifndef PROTO_OPS_H
+#define PROTO_OPS_H
+#include <stdint.h>
+#include <ArduinoJson.h>
+#include "ProtoSchema.h"
+#include "ShadeRequests.h"
+#include "SomfyFrame.h"
+
+// Socket protocol 2's command ops (docs/PROTOCOL.md): what each one asks for,
+// read from its payload.  Kept free of the shade model and the web server so
+// it can be unit tested on the PC (pio test -e native).
+//
+// The readers are strict where the HTTP routes guess: a field of the wrong
+// kind, a value out of range or a command name that isn't one is a bad
+// request, not a default.
+
+// Every command op except sendRemoteCommand becomes a shade request that
+// loop() runs.
+bool protoIsShadeOp(uint8_t type);
+// nullptr when the payload is good and req is filled in; otherwise what is
+// wrong with it, for a bad request reply.
+const char *parseShadeOp(uint8_t type, JsonObjectConst d, shade_request_t &req);
+// sendRemoteCommand: a frame as a remote that isn't one of the shades, so it
+// touches nothing of the shade model.  The same as parseShadeOp otherwise.
+const char *parseRemoteCommandOp(JsonObjectConst d, somfy_frame_t &frame, uint8_t &repeats);
+// A command by its name, as shadeCommanded reports it ("Up", "StepDown"),
+// in any case.  False for a name that isn't a command.
+bool protoCommand(const char *name, somfy_commands &cmd);
+
+// What a reply says when the radio didn't take a command.
+#define PROTO_RADIO_REFUSED "The radio didn't take the command"
+
+// The reply to a shade request that loop() has run: none when it was done.
+proto_error_t protoRequestError(shade_request_result_t result);
+// What that reply says.
+const char *protoRequestErrorDesc(const shade_request_t &req, shade_request_result_t result);
+#endif

src/ProtoSchema.h

@@ -24,6 +24,16 @@
   TYPE(syncEnd, 5, 'd') \
   TYPE(subscribe, 6, 'c') \
   TYPE(unsubscribe, 7, 'c') \
+  TYPE(shadeCommand, 16, 'c') \
+  TYPE(groupCommand, 17, 'c') \
+  TYPE(tiltCommand, 18, 'c') \
+  TYPE(repeatCommand, 19, 'c') \
+  TYPE(setPositions, 20, 'c') \
+  TYPE(setSensor, 21, 'c') \
+  TYPE(setMyPosition, 22, 'c') \
+  TYPE(unpairShade, 23, 'c') \
+  TYPE(setRollingCode, 24, 'c') \
+  TYPE(sendRemoteCommand, 25, 'c') \
   TYPE(shadeState, 128, 'd') \
   TYPE(shade, 129, 'd') \
   TYPE(shadeRemoved, 130, 'd') \
@@ -35,7 +45,7 @@
   TYPE(roomOrder, 136, 'd') \
   TYPE(shadeOrder, 137, 'd') \
   TYPE(groupOrder, 138, 'd') \
-  TYPE(shadeCommand, 139, 'd') \
+  TYPE(shadeCommanded, 139, 'd') \
   TYPE(memory, 140, 'd') \
   TYPE(wifi, 141, 'd') \
   TYPE(ethernet, 142, 'd') \
@@ -107,10 +117,13 @@
   FIELD(gpioLLTrigger, "gl", 'b', 0) \
   FIELD(linkedRemotes, "lk", 'l', 0) \
   FIELD(linkedShades, "ls", 'l', 0) \
-  /* shadeCommand */ \
+  /* the command ops and shadeCommanded */ \
   FIELD(command, "cm", 's', 0) \
   FIELD(source, "so", 's', 0) \
   FIELD(sourceAddress, "sr", 'i', 0) \
+  FIELD(repeat, "re", 'i', 0) \
+  FIELD(sunny, "sn", 'b', 0) \
+  FIELD(windy, "wn", 'b', 0) \
   /* the header */ \
   FIELD(maxRooms, "xr", 'i', 0) \
   FIELD(maxShades, "xs", 'i', 0) \

src/ShadeRequests.cpp

@@ -10,6 +10,16 @@ bool shadeRequestUsesRadio(shade_request_type_t type) {
       return true;
   }
 }
+bool shadeRequestIsForGroup(shade_request_type_t type) {
+  switch(type) {
+    case shade_request_type_t::group_command:
+    case shade_request_type_t::group_repeat:
+    case shade_request_type_t::group_sensor:
+      return true;
+    default:
+      return false;
+  }
+}
 int8_t requestRepeat(int32_t repeat) {
   if(repeat < 0) return -1;
   return repeat > INT8_MAX ? INT8_MAX : static_cast<int8_t>(repeat);
@@ -21,69 +31,69 @@ static bool inRange(int8_t pos) { return pos >= 0 && pos <= 100; }
 static int8_t storedPosition(int8_t pos, bool flip) { return displayPosition(static_cast<float>(pos), flip); }
 static uint8_t repeatsOr(int8_t repeat, uint8_t own) { return repeat >= 0 ? static_cast<uint8_t>(repeat) : own; }
 
-bool ShadeRequests::run(const shade_request_t &req) {
+static shade_request_result_t sent(bool taken) { return taken ? shade_request_result_t::done : shade_request_result_t::radio_refused; }
+
+shade_request_result_t ShadeRequests::run(const shade_request_t &req) {
   shade_info_t shade;
   uint8_t groupRepeats = 0;
-  switch(req.type) {
+  if(shadeRequestIsForGroup(req.type)) {
-    case shade_request_type_t::group_command:
+    if(!this->ops.findGroup(req.id, groupRepeats)) return shade_request_result_t::not_found;
-    case shade_request_type_t::group_repeat:
-    case shade_request_type_t::group_sensor:
-      if(!this->ops.findGroup(req.id, groupRepeats)) return true;
-      break;
-    default:
-      if(!this->ops.findShade(req.id, shade)) return true;
-      break;
   }
+  else if(!this->ops.findShade(req.id, shade)) return shade_request_result_t::not_found;
   const bool flip = shade.flipPosition;
   switch(req.type) {
     case shade_request_type_t::shade_command:
-      if(inRange(req.target)) return this->ops.moveToTarget(req.id, storedPosition(req.target, flip));
+      if(inRange(req.target)) return sent(this->ops.moveToTarget(req.id, storedPosition(req.target, flip)));
-      return this->ops.sendCommand(req.id, req.command, repeatsOr(req.repeat, shade.repeats), req.stepSize);
+      return sent(this->ops.sendCommand(req.id, req.command, repeatsOr(req.repeat, shade.repeats), req.stepSize));
     case shade_request_type_t::group_command:
-      return this->ops.sendGroupCommand(req.id, req.command, repeatsOr(req.repeat, groupRepeats), req.stepSize);
+      return sent(this->ops.sendGroupCommand(req.id, req.command, repeatsOr(req.repeat, groupRepeats), req.stepSize));
     case shade_request_type_t::tilt_command:
-      if(inRange(req.target)) return this->ops.moveToTiltTarget(req.id, storedPosition(req.target, flip));
+      if(inRange(req.target)) return sent(this->ops.moveToTiltTarget(req.id, storedPosition(req.target, flip)));
-      return this->ops.sendTiltCommand(req.id, req.command);
+      return sent(this->ops.sendTiltCommand(req.id, req.command));
     case shade_request_type_t::shade_repeat: {
       // A single button garage door has no Prog, its button is Toggle.
       somfy_commands cmd = req.command;
       if(shade.shadeType == shade_types::garage1 && cmd == somfy_commands::Prog) cmd = somfy_commands::Toggle;
       const uint8_t repeat = repeatsOr(req.repeat, shade.repeats);
-      if(this->ops.isLastCommand(req.id, cmd)) return this->ops.repeatFrame(req.id, repeat);
+      if(this->ops.isLastCommand(req.id, cmd)) return sent(this->ops.repeatFrame(req.id, repeat));
-      return this->ops.sendCommand(req.id, cmd, repeat, req.stepSize);
+      return sent(this->ops.sendCommand(req.id, cmd, repeat, req.stepSize));
     }
     case shade_request_type_t::group_repeat: {
       const uint8_t repeat = repeatsOr(req.repeat, groupRepeats);
-      if(this->ops.isLastGroupCommand(req.id, req.command)) return this->ops.repeatGroupFrame(req.id, repeat);
+      if(this->ops.isLastGroupCommand(req.id, req.command)) return sent(this->ops.repeatGroupFrame(req.id, repeat));
-      return this->ops.sendGroupCommand(req.id, req.command, repeat, req.stepSize);
+      return sent(this->ops.sendGroupCommand(req.id, req.command, repeat, req.stepSize));
     }
     case shade_request_type_t::set_positions: {
       const float pos = inRange(req.position) ? storedPosition(req.position, flip) : -1.0f;
       const float tilt = inRange(req.tiltPosition) && shade.tiltType != tilt_types::none ? storedPosition(req.tiltPosition, flip) : -1.0f;
+      // A tilt on its own for a shade without one has nothing to set.
+      if(pos < 0.0f && inRange(req.tiltPosition) && shade.tiltType == tilt_types::none) return shade_request_result_t::no_tilt;
       if(pos >= 0.0f || tilt >= 0.0f) this->ops.setPositions(req.id, pos, tilt);
-      return true;
+      return shade_request_result_t::done;
     }
     case shade_request_type_t::shade_sensor:
-      return this->ops.sendSensorCommand(req.id, req.windy, req.sunny, repeatsOr(req.repeat, shade.repeats));
+      return sent(this->ops.sendSensorCommand(req.id, req.windy, req.sunny, repeatsOr(req.repeat, shade.repeats)));
     case shade_request_type_t::group_sensor:
-      return this->ops.sendGroupSensorCommand(req.id, req.windy, req.sunny, repeatsOr(req.repeat, groupRepeats));
+      return sent(this->ops.sendGroupSensorCommand(req.id, req.windy, req.sunny, repeatsOr(req.repeat, groupRepeats)));
     case shade_request_type_t::set_favorite: {
+      // The shade only takes a favorite at rest, and would ignore it.
+      if(!shade.idle) return shade_request_result_t::shade_moving;
       const int8_t tilt = requestedFavoriteTilt(req.tiltPosition, shade.tiltType, shade.myTiltPos, flip);
       // Without a lift position only the tilt changes: the lift favorite stays,
       // or is set where the shade is when it has none.
       int8_t pos = req.position;
       if(!inRange(pos)) pos = displayPosition(shade.myPos >= 0.0f ? shade.myPos : shade.currentPos, flip);
-      return this->ops.setMyPosition(req.id, storedPosition(pos, flip), storedPosition(tilt, flip));
+      return sent(this->ops.setMyPosition(req.id, storedPosition(pos, flip), storedPosition(tilt, flip)));
     }
     case shade_request_type_t::unpair: {
       // A 56-bit motor needs a longer Prog press than an 80-bit one.
-      const bool sent = this->ops.sendCommand(req.id, somfy_commands::Prog, shade.bitLength == 56 ? 7 : 1, 0);
+      const bool taken = this->ops.sendCommand(req.id, somfy_commands::Prog, shade.bitLength == 56 ? 7 : 1, 0);
-      if(sent) this->ops.setUnpaired(req.id);
+      if(taken) this->ops.setUnpaired(req.id);
-      return sent;
+      return sent(taken);
     }
     case shade_request_type_t::set_rolling_code:
       this->ops.setRollingCode(req.id, req.rollingCode);
-      return true;
+      return shade_request_result_t::done;
   }
-  return true;
+  return shade_request_result_t::done;
 }

src/ShadeRequests.h

@@ -40,6 +40,8 @@ struct shade_request_t {
 
 // Whether the request sends anything on air.
 bool shadeRequestUsesRadio(shade_request_type_t type);
+// Whether the request's id is a group id rather than a shade id.
+bool shadeRequestIsForGroup(shade_request_type_t type);
 // A repeat count from a client: negative for the default, capped at what fits.
 int8_t requestRepeat(int32_t repeat);
 // A target from a client: -1 unless it is 0-100.
@@ -55,6 +57,7 @@ struct shade_info_t {
   float currentPos = 0.0f;
   float myPos = -1.0f;
   float myTiltPos = -1.0f;
+  bool idle = true;  // not moving, and at its target
 };
 
 // The seam between ShadeRequests and the shades and groups it acts on.  On the
@@ -85,16 +88,22 @@ class ShadeOps {
     virtual bool sendGroupSensorCommand(uint8_t groupId, int8_t windy, int8_t sunny, uint8_t repeat) = 0;
 };
 
+// How a request went.  One with nothing in range to do is done.
+enum class shade_request_result_t : uint8_t {
+  done,
+  not_found,      // no shade or group with that id; nothing was done
+  radio_refused,  // the radio didn't take a command it sent
+  shade_moving,   // a favorite is only set on a shade at rest; nothing was done
+  no_tilt         // only a tilt was given, for a shade without one; nothing was done
+};
+
 // Carries out shade requests, whichever entry point they came from: finds
 // the shade or group, turns client positions into stored ones, checks
 // ranges and fills in default repeats.  Runs on loop() only.
 class ShadeRequests {
   public:
     explicit ShadeRequests(ShadeOps &ops) : ops(ops) {}
-    // False when the radio refused a command it sent.  A request for a shade
+    shade_request_result_t run(const shade_request_t &req);
-    // or group that doesn't exist, or with nothing in range to do, is dropped
-    // and returns true.
-    bool run(const shade_request_t &req);
   private:
     ShadeOps &ops;
 };

src/Somfy.h

@@ -105,6 +105,7 @@ class SomfyShade : public SomfyRemote, public ShadeSettings, private MotorLink,
     uint8_t getShadeId() { return shadeId; }
     // Where the shade is and where it is going.
     const ShadeBehaviour::State &state() const { return this->behaviour.state(); }
+    bool isIdle() const { return this->behaviour.isIdle(); }
     uint8_t getFlags() override;
     // Sets the setting and state bits from a saved or sent flags byte.
     void setFlags(uint8_t flags);

src/SomfyShadeOps.cpp

@@ -16,6 +16,7 @@ bool SomfyShadeOps::findShade(uint8_t shadeId, shade_info_t &info) {
   info.currentPos = shade->state().currentPos;
   info.myPos = shade->state().myPos;
   info.myTiltPos = shade->state().myTiltPos;
+  info.idle = shade->isIdle();
   return true;
 }
 bool SomfyShadeOps::findGroup(uint8_t groupId, uint8_t &repeats) {

src/outputs/ProtoSocket.cpp

@@ -7,6 +7,8 @@
 #include "ProtoFrame.h"
 #include "ProtoParse.h"
 #include "ProtoSlots.h"
+#include "ProtoOps.h"
+#include "LoopQueue.h"
 #include "SomfySnapshot.h"
 #include "Somfy.h"
 #include "ConfigSettings.h"
@@ -26,6 +28,7 @@ extern ESPNetwork net;
 extern Web webServer;
 extern LoopStats loopStats;
 extern SocketEmitter sockEmit;
+extern LoopQueue loopQueue;
 
 ProtoSocket protoSocket;
 static AsyncWebSocket ws("/ws");
@@ -144,6 +147,34 @@ static void replyOk(AsyncWebSocketClient *client, uint32_t id) {
   if(id == 0) return;
   sendFixed(client, [](JsonFormatter &json, void *ctx) { writeOkReply(json, *static_cast<uint32_t *>(ctx)); }, &id);
 }
+// A command op that becomes a shade request: loop() runs it and replies.
+static void queueShadeOp(AsyncWebSocketClient *client, const proto_message_t &req, JsonObjectConst d) {
+  loop_item_t item;
+  const char *bad = parseShadeOp(req.type, d, item.request);
+  if(bad) { replyError(client, req.id, proto_error_t::badRequest, bad); return; }
+  // Nothing goes on air during a frequency scan, so say so now rather than
+  // once it reaches the radio.
+  if(loopItemUsesRadio(item) && somfy.transceiver.txSuspended()) {
+    replyError(client, req.id, proto_error_t::radioUnavailable, "The radio is paused");
+    return;
+  }
+  item.replyClient = client->id();
+  item.replyId = req.id;
+  if(!loopQueue.submit(item)) replyError(client, req.id, proto_error_t::busy, "Command queue full");
+}
+// A frame as a remote that isn't one of the shades.  It touches nothing of the
+// shade model and the radio takes jobs from any task, so it goes from here.
+static void sendRemoteCommand(AsyncWebSocketClient *client, const proto_message_t &req, JsonObjectConst d) {
+  somfy_frame_t frame;
+  uint8_t repeats;
+  const char *bad = parseRemoteCommandOp(d, frame, repeats);
+  if(bad) { replyError(client, req.id, proto_error_t::badRequest, bad); return; }
+  if(!somfy.sendFrame(frame, repeats)) {
+    replyError(client, req.id, proto_error_t::radioUnavailable, PROTO_RADIO_REFUSED);
+    return;
+  }
+  replyOk(client, req.id);
+}
 static bool authorized(AsyncWebServerRequest *request) {
   if(settings.Security.type == security_types::None) return true;
   const AsyncWebParameter *key = request->getParam("apikey");
@@ -210,8 +241,15 @@ static void onFrame(AsyncWebSocketClient *client, AwsFrameInfo *info, uint8_t *d
       replyOk(client, msg.id);
       return;
     }
+    case proto_type_t::sendRemoteCommand:
+      sendRemoteCommand(client, msg, doc["d"]);
+      return;
     default:
-      // The ops arrive with the protocol core's later PRs.
+      if(protoIsShadeOp(msg.type)) {
+        queueShadeOp(client, msg, doc["d"]);
+        return;
+      }
+      // The edit and settings ops arrive with the protocol core's later PRs.
       replyError(client, msg.id, proto_error_t::unknownOp, "Unknown message type");
       return;
   }
@@ -271,6 +309,18 @@ void ProtoSocket::loop() {
   ws.cleanupClients(PROTO_MAX_CLIENTS);
 }
 
+void ProtoSocket::replyRequest(uint32_t client, uint32_t id, const shade_request_t &req, shade_request_result_t result) {
+  if(!sockEmit.onLoopTask() || id == 0) return;
+  char buf[192];
+  BufferedJsonFormatter json;
+  json.setBuffer(buf, sizeof(buf));
+  const proto_error_t code = protoRequestError(result);
+  if(code == proto_error_t::none) writeOkReply(json, id);
+  else writeErrorReply(json, id, code, protoRequestErrorDesc(req, result));
+  // A client that has gone is skipped.
+  if(!json.overflowed()) ws.text(client, buf, json.length());
+}
+
 /*********************************************************************
  * loop(): events
  ********************************************************************/
@@ -373,9 +423,9 @@ void ProtoSocket::repeaters() {
   w.endList();
   sendEvent();
 }
-void ProtoSocket::shadeCommand(SomfyShade &shade, const char *command, const char *source, uint32_t sourceAddress) {
+void ProtoSocket::shadeCommanded(SomfyShade &shade, const char *command, const char *source, uint32_t sourceAddress) {
   if(!this->sending()) return;
-  ProtoWriter w(beginEvent(proto_type_t::shadeCommand));
+  ProtoWriter w(beginEvent(proto_type_t::shadeCommanded));
   w.add(proto_field_t::shadeId, shade.getShadeId());
   w.add(proto_field_t::remoteAddress, shade.getRemoteAddress());
   w.add(proto_field_t::command, command);

src/outputs/ProtoSocket.h

@@ -3,6 +3,7 @@
 #include <ESPAsyncWebServer.h>
 #include "ProtoSchema.h"
 #include "ModelEditor.h"
+#include "ShadeRequests.h"
 
 class SomfyShade;
 class SomfyGroup;
@@ -23,6 +24,8 @@ class ProtoSocket {
     // Moves each client's sync on.  loop() only.
     void loop();
     bool hasClients() const;
+    // The reply to a command op once loop() has run its shade request.
+    void replyRequest(uint32_t client, uint32_t id, const shade_request_t &req, shade_request_result_t result);
     // Events, loop() only.
     void shadeState(SomfyShade &shade);
     void shade(SomfyShade &shade);
@@ -35,7 +38,7 @@ class ProtoSocket {
     // roomOrder, shadeOrder or groupOrder: the list's ids, in sort order.
     void order(sort_list_t list);
     void repeaters();
-    void shadeCommand(SomfyShade &shade, const char *command, const char *source, uint32_t sourceAddress);
+    void shadeCommanded(SomfyShade &shade, const char *command, const char *source, uint32_t sourceAddress);
     void memory();
     void wifi();
     void ethernet();

src/outputs/SomfyEvents.cpp

@@ -50,7 +50,7 @@ void SomfyShade::emitCommand(uint8_t num, somfy_commands cmd, const char *source
   json->addElem("sourceAddress", (uint32_t)sourceAddress);
   json->endObject();
   sockEmit.endEmit(num);
-  if(num == 255) protoSocket.shadeCommand(*this, translateSomfyCommand(cmd).c_str(), source, sourceAddress);
+  if(num == 255) protoSocket.shadeCommanded(*this, translateSomfyCommand(cmd).c_str(), source, sourceAddress);
   if(mqtt.connected()) {
     this->publish("cmdSource", source);
     this->publish("cmdAddress", sourceAddress);

test/e2e/README.md

@@ -10,8 +10,9 @@ Runs against a real device on your network, by hand, never in CI. It checks sock
 |---|---|
 | `--device` | the device's address (required) |
 | `--password`, `--username` / `--pin` | the credentials of a Secured device |
+| `--radio --shade <id>` | also send real commands: that shade moves a little and back |
 | `--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 settings and doesn't reboot the device. It does fill every socket slot for a few seconds, and replays a page load.
+A plain run sends nothing on air, changes no settings 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. It does fill every socket slot for a few seconds, and replays a page load.
 
 The tests run in whatever security mode the device is in; the suite never changes it. To test both modes, run the suite once in each.

test/e2e/conftest.py

@@ -15,6 +15,8 @@ def pytest_addoption(parser):
     parser.addoption("--username", default="", help="the username that goes with --password")
     parser.addoption("--pin", default=None, help="the device's PIN, when it is Secured with one")
     parser.addoption("--record", action="store_true", help="save what the device sends to test/fixtures/proto2")
+    parser.addoption("--radio", action="store_true", help="also run the tests that send commands on air")
+    parser.addoption("--shade", type=int, default=None, help="with --radio: the id of the shade the commands may move")
 
 
 @pytest.fixture(scope="session")
@@ -82,3 +84,14 @@ def schema(host, token):
 @pytest.fixture(scope="session")
 def recording(request):
     return request.config.getoption("--record")
+
+
+@pytest.fixture(scope="session")
+def radio_shade(request):
+    """The shade --radio tests may move, or a skip."""
+    if not request.config.getoption("--radio"):
+        pytest.skip("sends on air: pass --radio --shade <id>")
+    shade = request.config.getoption("--shade")
+    if shade is None:
+        pytest.skip("--radio needs --shade <id>, a shade that may move")
+    return shade

test/e2e/test_commands.py

@@ -0,0 +1,112 @@
+"""The command ops (docs/PROTOCOL.md).  Without --radio nothing goes on air:
+the ops are refused before they reach the radio, or send nothing anyway."""
+import time
+
+import pytest
+
+from proto_client import run_sync
+from test_messages import error_name, reply_to
+
+COMMAND_OPS = ("shadeCommand", "groupCommand", "tiltCommand", "repeatCommand", "setPositions", "setSensor",
+               "setMyPosition", "unpairShade", "setRollingCode", "sendRemoteCommand")
+
+
+def pack(schema, fields):
+    """A payload by field names, in short keys."""
+    return {schema.keys[name]: value for name, value in fields.items()}
+
+
+def synced_model(connect, schema):
+    """A synced client and the shades and groups it got, by name."""
+    c = connect(schema=schema.hash)
+    _, frames, _ = run_sync(c)
+    shades, groups = [], []
+    for f in frames:
+        shades.extend(schema.expand(s) for s in f["d"].get(schema.keys["shades"], []))
+        groups.extend(schema.expand(g) for g in f["d"].get(schema.keys["groups"], []))
+    return c, shades, groups
+
+
+def op(c, schema, name, fields):
+    """Sends the op and returns its reply."""
+    msg_id = c.send(schema.type_numbers[name], pack(schema, fields), with_id=True)
+    return reply_to(c, msg_id)
+
+
+def is_ok(schema, reply):
+    return reply["d"].get(schema.keys["ok"]) == 1
+
+
+def test_every_command_op_is_in_the_schema(schema):
+    for name in COMMAND_OPS:
+        assert 16 <= schema.type_numbers[name] <= 127, name
+
+
+def test_an_op_without_an_id_is_a_bad_request(connect, schema):
+    c, _, _ = synced_model(connect, schema)
+    c.send(schema.type_numbers["shadeCommand"], pack(schema, {"shadeId": 1, "command": "My"}))
+    reply = c.recv_until(lambda f: f["t"] == 2)[-1]
+    assert "id" not in reply
+    assert error_name(schema, reply) == "badRequest"
+
+
+@pytest.mark.parametrize("name, fields, why", [
+    ("shadeCommand", {}, "shadeId is missing"),
+    ("shadeCommand", {"shadeId": 1, "command": "Upp"}, "Not a command"),
+    ("shadeCommand", {"shadeId": 1}, "Give command or target"),
+    ("shadeCommand", {"shadeId": 1, "target": 101}, "target must be 0-100"),
+    ("tiltCommand", {"shadeId": 1, "command": "Up", "target": 5}, "Give command or target"),
+    ("groupCommand", {"groupId": 1}, "command is missing"),
+    ("repeatCommand", {"command": "Up"}, "Give shadeId or groupId"),
+    ("setPositions", {"shadeId": 1}, "Give position, tiltPosition or both"),
+    ("setSensor", {"shadeId": 1}, "Give sunny, windy or both"),
+    ("setMyPosition", {"shadeId": 1, "myPos": 101}, "myPos must be 0-100"),
+    ("unpairShade", {}, "shadeId is missing"),
+    ("setRollingCode", {"shadeId": 1}, "lastRollingCode is missing"),
+    ("sendRemoteCommand", {"command": "Up", "lastRollingCode": 1}, "remoteAddress is missing"),
+])
+def test_a_bad_payload_is_refused_with_why(connect, schema, name, fields, why):
+    c, _, _ = synced_model(connect, schema)
+    reply = op(c, schema, name, fields)
+    assert error_name(schema, reply) == "badRequest"
+    assert reply["d"][schema.keys["desc"]] == why
+
+
+def test_a_shade_or_group_that_does_not_exist_is_not_found(connect, schema):
+    c, shades, groups = synced_model(connect, schema)
+    shade = next(i for i in range(254, 0, -1) if i not in {s["shadeId"] for s in shades})
+    group = next(i for i in range(254, 0, -1) if i not in {g["groupId"] for g in groups})
+    reply = op(c, schema, "shadeCommand", {"shadeId": shade, "command": "Up"})
+    assert error_name(schema, reply) == "notFound"
+    assert reply["d"][schema.keys["desc"]] == "No shade with that id"
+    reply = op(c, schema, "groupCommand", {"groupId": group, "command": "Up"})
+    assert error_name(schema, reply) == "notFound"
+    assert reply["d"][schema.keys["desc"]] == "No group with that id"
+
+
+def test_set_positions_where_the_shade_already_is(connect, schema):
+    """setPositions sends nothing on air.  Setting the position the sync gave
+    moves the shade's believed position by less than 1%, its fraction."""
+    c, shades, _ = synced_model(connect, schema)
+    if not shades:
+        pytest.skip("the device has no shades")
+    shade = shades[0]
+    reply = op(c, schema, "setPositions", {"shadeId": shade["shadeId"], "position": shade.get("position", 0)})
+    assert is_ok(schema, reply)
+
+
+def test_a_command_moves_the_shade_and_replies_ok(connect, schema, radio_shade):
+    """Moves the --shade a little and back to where it was."""
+    c, shades, _ = synced_model(connect, schema)
+    shade = next((s for s in shades if s["shadeId"] == radio_shade), None)
+    assert shade, "no shade %s on the device" % radio_shade
+    start = shade.get("position", 0)
+    reply = op(c, schema, "shadeCommand", {"shadeId": radio_shade, "command": "Down" if start < 50 else "Up"})
+    assert is_ok(schema, reply)
+    state = schema.type_numbers["shadeState"]
+    moving = c.recv_until(lambda f: f["t"] == state and f["d"].get(schema.keys["shadeId"]) == radio_shade
+                          and f["d"].get(schema.keys["direction"], 0) != 0, timeout=10)[-1]
+    assert moving
+    time.sleep(2)
+    reply = op(c, schema, "shadeCommand", {"shadeId": radio_shade, "target": start})
+    assert is_ok(schema, reply)

test/fixtures/proto2/hello.json

@@ -3,6 +3,6 @@
  "t": 1,
  "d": {
   "fw": "v3.0.14",
-  "sh": "8198c564"
+  "sh": "0fa739cc"
  }
 }

test/fixtures/proto2/schema.json

@@ -1,5 +1,5 @@
 {
- "hash": "8198c564",
+ "hash": "0fa739cc",
  "types": {
   "1": "hello",
   "2": "reply",
@@ -8,6 +8,16 @@
   "5": "syncEnd",
   "6": "subscribe",
   "7": "unsubscribe",
+  "16": "shadeCommand",
+  "17": "groupCommand",
+  "18": "tiltCommand",
+  "19": "repeatCommand",
+  "20": "setPositions",
+  "21": "setSensor",
+  "22": "setMyPosition",
+  "23": "unpairShade",
+  "24": "setRollingCode",
+  "25": "sendRemoteCommand",
   "128": "shadeState",
   "129": "shade",
   "130": "shadeRemoved",
@@ -19,7 +29,7 @@
   "136": "roomOrder",
   "137": "shadeOrder",
   "138": "groupOrder",
-  "139": "shadeCommand",
+  "139": "shadeCommanded",
   "140": "memory",
   "141": "wifi",
   "142": "ethernet",
@@ -333,6 +343,11 @@
    "kind": "i",
    "default": 0
   },
+  "re": {
+   "name": "repeat",
+   "kind": "i",
+   "default": 0
+  },
   "rp": {
    "name": "repeats",
    "kind": "i",
@@ -383,6 +398,11 @@
    "kind": "b",
    "default": 0
   },
+  "sn": {
+   "name": "sunny",
+   "kind": "b",
+   "default": 0
+  },
   "so": {
    "name": "source",
    "kind": "s",
@@ -453,6 +473,11 @@
    "kind": "i",
    "default": 0
   },
+  "wn": {
+   "name": "windy",
+   "kind": "b",
+   "default": 0
+  },
   "x": {
    "name": "desc",
    "kind": "s",

test/fixtures/proto2/sync.json

@@ -32,6 +32,46 @@
      7,
      "unsubscribe"
     ],
+    [
+     16,
+     "shadeCommand"
+    ],
+    [
+     17,
+     "groupCommand"
+    ],
+    [
+     18,
+     "tiltCommand"
+    ],
+    [
+     19,
+     "repeatCommand"
+    ],
+    [
+     20,
+     "setPositions"
+    ],
+    [
+     21,
+     "setSensor"
+    ],
+    [
+     22,
+     "setMyPosition"
+    ],
+    [
+     23,
+     "unpairShade"
+    ],
+    [
+     24,
+     "setRollingCode"
+    ],
+    [
+     25,
+     "sendRemoteCommand"
+    ],
     [
      128,
      "shadeState"
@@ -78,7 +118,7 @@
     ],
     [
      139,
-     "shadeCommand"
+     "shadeCommanded"
     ],
     [
      140,
@@ -454,6 +494,24 @@
      "i",
      0
     ],
+    [
+     "re",
+     "repeat",
+     "i",
+     0
+    ],
+    [
+     "sn",
+     "sunny",
+     "b",
+     0
+    ],
+    [
+     "wn",
+     "windy",
+     "b",
+     0
+    ],
     [
      "xr",
      "maxRooms",
@@ -845,9 +903,19 @@
        "c": 1048614
       }
      ]
-    },
+    }
+   ]
+  }
+ },
+ {
+  "v": 2,
+  "t": 4,
+  "d": {
+   "S": [
     {
      "s": 6,
+     "p": 100,
+     "t": 100,
      "c": 1048615,
      "n": "name 40",
      "a": 1048617,
@@ -895,15 +963,7 @@
        "c": 1048628
       }
      ]
-    }
+    },
-   ]
-  }
- },
- {
-  "v": 2,
-  "t": 4,
-  "d": {
-   "S": [
     {
      "s": 8,
      "c": 1048629,
@@ -1020,15 +1080,13 @@
     }
    ],
    "Z": {
-    "fr": 94956,
+    "fr": 106640,
-    "mx": 53236,
+    "mx": 49140,
-    "mn": 45904,
+    "mn": 61988,
-    "to": 266884,
+    "to": 266820,
-    "up": 672549,
+    "up": 29406,
-    "lp": 998,
-    "lx": 13409,
     "sd": "ssid 80",
-    "sg": -68,
+    "sg": -69,
     "ch": 6
    }
   }

test/test_proto/test_proto.cpp

@@ -45,6 +45,11 @@ static void test_type_numbers_are_unique_and_in_their_sender_range() {
     else TEST_ASSERT_TRUE_MESSAGE(t.number >= 1, t.name);
   }
 }
+static void test_type_names_are_unique() {
+  // Clients look types up by name.
+  std::set<std::string> names;
+  for(uint8_t i = 0; i < protoTypeCount(); i++) TEST_ASSERT_TRUE_MESSAGE(names.insert(protoTypeAt(i).name).second, protoTypeAt(i).name);
+}
 static void test_error_and_feed_numbers_are_unique() {
   std::set<uint8_t> errors, feeds;
   for(uint8_t i = 0; i < protoErrorCount(); i++) TEST_ASSERT_TRUE(errors.insert(protoErrorAt(i).number).second);
@@ -380,6 +385,7 @@ int main(int argc, char **argv) {
   RUN_TEST(test_field_keys_and_names_are_unique);
   RUN_TEST(test_every_field_has_a_known_kind);
   RUN_TEST(test_type_numbers_are_unique_and_in_their_sender_range);
+  RUN_TEST(test_type_names_are_unique);
   RUN_TEST(test_error_and_feed_numbers_are_unique);
   RUN_TEST(test_bootstrap_keys_are_fixed);
   RUN_TEST(test_schema_hash_is_eight_hex_digits_and_stable);

test/test_protoops/test_protoops.cpp

@@ -0,0 +1,252 @@
+#include <unity.h>
+#include <string.h>
+#include <ArduinoJson.h>
+#include "ProtoOps.h"
+
+// Socket protocol 2's command ops: what each payload asks for, and what is
+// refused before anything reaches loop() (docs/PROTOCOL.md).
+
+void setUp() {}
+void tearDown() {}
+
+static JsonDocument doc;
+static shade_request_t req;
+
+static const char *parse(proto_type_t type, const char *payload) {
+  doc.clear();
+  TEST_ASSERT_EQUAL(DeserializationError::Ok, deserializeJson(doc, payload).code());
+  return parseShadeOp(static_cast<uint8_t>(type), doc.as<JsonObjectConst>(), req);
+}
+#define TEST_ASSERT_GOOD(why) TEST_ASSERT_NULL_MESSAGE(why, why)
+
+/*********************************************************************
+ * Which ops are shade requests
+ ********************************************************************/
+static void test_every_command_op_but_the_remote_frame_is_a_shade_request() {
+  const proto_type_t shadeOps[] = {
+    proto_type_t::shadeCommand, proto_type_t::groupCommand, proto_type_t::tiltCommand, proto_type_t::repeatCommand,
+    proto_type_t::setPositions, proto_type_t::setSensor, proto_type_t::setMyPosition, proto_type_t::unpairShade,
+    proto_type_t::setRollingCode
+  };
+  for(proto_type_t t : shadeOps) TEST_ASSERT_TRUE(protoIsShadeOp(static_cast<uint8_t>(t)));
+  TEST_ASSERT_FALSE(protoIsShadeOp(static_cast<uint8_t>(proto_type_t::sendRemoteCommand)));
+  TEST_ASSERT_FALSE(protoIsShadeOp(static_cast<uint8_t>(proto_type_t::subscribe)));
+  TEST_ASSERT_FALSE(protoIsShadeOp(static_cast<uint8_t>(proto_type_t::shadeCommanded)));
+}
+
+/*********************************************************************
+ * Commands
+ ********************************************************************/
+static void test_shade_command_by_name() {
+  TEST_ASSERT_GOOD(parse(proto_type_t::shadeCommand, R"({"s":3,"cm":"Down"})"));
+  TEST_ASSERT_EQUAL(shade_request_type_t::shade_command, req.type);
+  TEST_ASSERT_EQUAL_UINT8(3, req.id);
+  TEST_ASSERT_EQUAL(somfy_commands::Down, req.command);
+  TEST_ASSERT_EQUAL_INT8(-1, req.target);
+  // Left out: the shade's own repeats.
+  TEST_ASSERT_EQUAL_INT8(-1, req.repeat);
+}
+static void test_shade_command_to_a_target() {
+  TEST_ASSERT_GOOD(parse(proto_type_t::shadeCommand, R"({"s":3,"t":0})"));
+  TEST_ASSERT_EQUAL_INT8(0, req.target);
+}
+static void test_shade_command_with_repeat_and_step() {
+  TEST_ASSERT_GOOD(parse(proto_type_t::shadeCommand, R"({"s":3,"cm":"StepUp","re":0,"z":5})"));
+  TEST_ASSERT_EQUAL(somfy_commands::StepUp, req.command);
+  TEST_ASSERT_EQUAL_INT8(0, req.repeat);
+  TEST_ASSERT_EQUAL_UINT8(5, req.stepSize);
+}
+static void test_shade_command_needs_a_command_or_a_target_not_both() {
+  TEST_ASSERT_EQUAL_STRING("Give command or target", parse(proto_type_t::shadeCommand, R"({"s":3})"));
+  TEST_ASSERT_EQUAL_STRING("Give command or target", parse(proto_type_t::shadeCommand, R"({"s":3,"cm":"Up","t":10})"));
+}
+static void test_shade_command_values_are_checked() {
+  TEST_ASSERT_EQUAL_STRING("shadeId is missing", parse(proto_type_t::shadeCommand, R"({"cm":"Up"})"));
+  TEST_ASSERT_EQUAL_STRING("shadeId must be 0-255", parse(proto_type_t::shadeCommand, R"({"s":"3","cm":"Up"})"));
+  TEST_ASSERT_EQUAL_STRING("target must be 0-100", parse(proto_type_t::shadeCommand, R"({"s":3,"t":101})"));
+  TEST_ASSERT_EQUAL_STRING("Not a command", parse(proto_type_t::shadeCommand, R"({"s":3,"cm":"Upp"})"));
+  TEST_ASSERT_EQUAL_STRING("Not a command", parse(proto_type_t::shadeCommand, R"({"s":3,"cm":2})"));
+  TEST_ASSERT_EQUAL_STRING("repeat must be 0-127", parse(proto_type_t::shadeCommand, R"({"s":3,"cm":"Up","re":-1})"));
+  TEST_ASSERT_EQUAL_STRING("stepSize must be 0-127", parse(proto_type_t::shadeCommand, R"({"s":3,"cm":"Up","z":128})"));
+}
+static void test_only_the_first_problem_is_reported() {
+  TEST_ASSERT_EQUAL_STRING("shadeId is missing", parse(proto_type_t::shadeCommand, R"({"t":101,"re":-1})"));
+}
+static void test_a_left_out_payload_is_missing_everything() {
+  doc.clear();
+  TEST_ASSERT_EQUAL_STRING("shadeId is missing", parseShadeOp(static_cast<uint8_t>(proto_type_t::unpairShade), JsonObjectConst(), req));
+}
+static void test_tilt_command() {
+  TEST_ASSERT_GOOD(parse(proto_type_t::tiltCommand, R"({"s":2,"t":40})"));
+  TEST_ASSERT_EQUAL(shade_request_type_t::tilt_command, req.type);
+  TEST_ASSERT_EQUAL_INT8(40, req.target);
+}
+static void test_group_command_needs_a_command() {
+  TEST_ASSERT_GOOD(parse(proto_type_t::groupCommand, R"({"g":1,"cm":"my","re":4})"));
+  TEST_ASSERT_EQUAL(shade_request_type_t::group_command, req.type);
+  TEST_ASSERT_EQUAL_UINT8(1, req.id);
+  TEST_ASSERT_EQUAL(somfy_commands::My, req.command);
+  TEST_ASSERT_EQUAL_INT8(4, req.repeat);
+  TEST_ASSERT_EQUAL_STRING("command is missing", parse(proto_type_t::groupCommand, R"({"g":1,"t":10})"));
+}
+static void test_repeat_takes_a_shade_or_a_group() {
+  TEST_ASSERT_GOOD(parse(proto_type_t::repeatCommand, R"({"s":5,"cm":"Prog"})"));
+  TEST_ASSERT_EQUAL(shade_request_type_t::shade_repeat, req.type);
+  TEST_ASSERT_EQUAL_UINT8(5, req.id);
+  TEST_ASSERT_GOOD(parse(proto_type_t::repeatCommand, R"({"g":2,"cm":"Up"})"));
+  TEST_ASSERT_EQUAL(shade_request_type_t::group_repeat, req.type);
+  TEST_ASSERT_EQUAL_UINT8(2, req.id);
+  TEST_ASSERT_EQUAL_STRING("Give shadeId or groupId", parse(proto_type_t::repeatCommand, R"({"s":5,"g":2,"cm":"Up"})"));
+  TEST_ASSERT_EQUAL_STRING("Give shadeId or groupId", parse(proto_type_t::repeatCommand, R"({"cm":"Up"})"));
+}
+static void test_command_names_read_back_as_shade_commanded_reports_them() {
+  somfy_commands cmd;
+  TEST_ASSERT_TRUE(protoCommand("Step Down", cmd));
+  TEST_ASSERT_EQUAL(somfy_commands::StepDown, cmd);
+  TEST_ASSERT_TRUE(protoCommand("My+Up+Down", cmd));
+  TEST_ASSERT_EQUAL(somfy_commands::MyUpDown, cmd);
+  TEST_ASSERT_TRUE(protoCommand("SUNFLAG", cmd));
+  TEST_ASSERT_EQUAL(somfy_commands::SunFlag, cmd);
+  // The HTTP routes' guesses from the first letter are not names.
+  TEST_ASSERT_FALSE(protoCommand("u", cmd));
+  TEST_ASSERT_FALSE(protoCommand("", cmd));
+  TEST_ASSERT_FALSE(protoCommand(nullptr, cmd));
+}
+
+/*********************************************************************
+ * Positions, sensors, favorites, pairing
+ ********************************************************************/
+static void test_set_positions_leaves_out_what_was_not_given() {
+  TEST_ASSERT_GOOD(parse(proto_type_t::setPositions, R"({"s":1,"p":0})"));
+  TEST_ASSERT_EQUAL(shade_request_type_t::set_positions, req.type);
+  TEST_ASSERT_EQUAL_INT8(0, req.position);
+  TEST_ASSERT_EQUAL_INT8(-1, req.tiltPosition);
+  TEST_ASSERT_GOOD(parse(proto_type_t::setPositions, R"({"s":1,"q":30})"));
+  TEST_ASSERT_EQUAL_INT8(-1, req.position);
+  TEST_ASSERT_EQUAL_INT8(30, req.tiltPosition);
+  TEST_ASSERT_EQUAL_STRING("Give position, tiltPosition or both", parse(proto_type_t::setPositions, R"({"s":1})"));
+  TEST_ASSERT_EQUAL_STRING("position must be 0-100", parse(proto_type_t::setPositions, R"({"s":1,"p":-1})"));
+}
+static void test_sensor_leaves_out_what_was_not_given() {
+  TEST_ASSERT_GOOD(parse(proto_type_t::setSensor, R"({"g":3,"sn":1})"));
+  TEST_ASSERT_EQUAL(shade_request_type_t::group_sensor, req.type);
+  TEST_ASSERT_EQUAL_INT8(1, req.sunny);
+  TEST_ASSERT_EQUAL_INT8(-1, req.windy);
+  TEST_ASSERT_GOOD(parse(proto_type_t::setSensor, R"({"s":3,"wn":0,"re":2})"));
+  TEST_ASSERT_EQUAL(shade_request_type_t::shade_sensor, req.type);
+  TEST_ASSERT_EQUAL_INT8(-1, req.sunny);
+  TEST_ASSERT_EQUAL_INT8(0, req.windy);
+  TEST_ASSERT_EQUAL_INT8(2, req.repeat);
+  TEST_ASSERT_EQUAL_STRING("Give sunny, windy or both", parse(proto_type_t::setSensor, R"({"s":3})"));
+  // Booleans are 1 and 0.
+  TEST_ASSERT_EQUAL_STRING("sunny must be 0-1", parse(proto_type_t::setSensor, R"({"s":3,"sn":true})"));
+}
+static void test_favorite() {
+  TEST_ASSERT_GOOD(parse(proto_type_t::setMyPosition, R"({"s":4,"m":50,"M":20})"));
+  TEST_ASSERT_EQUAL(shade_request_type_t::set_favorite, req.type);
+  TEST_ASSERT_EQUAL_INT8(50, req.position);
+  TEST_ASSERT_EQUAL_INT8(20, req.tiltPosition);
+  TEST_ASSERT_GOOD(parse(proto_type_t::setMyPosition, R"({"s":4,"M":20})"));
+  TEST_ASSERT_EQUAL_INT8(-1, req.position);
+  TEST_ASSERT_EQUAL_STRING("Give myPos, myTiltPos or both", parse(proto_type_t::setMyPosition, R"({"s":4})"));
+}
+static void test_unpair_and_rolling_code() {
+  TEST_ASSERT_GOOD(parse(proto_type_t::unpairShade, R"({"s":6})"));
+  TEST_ASSERT_EQUAL(shade_request_type_t::unpair, req.type);
+  TEST_ASSERT_EQUAL_UINT8(6, req.id);
+  TEST_ASSERT_GOOD(parse(proto_type_t::setRollingCode, R"({"s":6,"c":0})"));
+  TEST_ASSERT_EQUAL(shade_request_type_t::set_rolling_code, req.type);
+  TEST_ASSERT_EQUAL_UINT16(0, req.rollingCode);
+  TEST_ASSERT_EQUAL_STRING("lastRollingCode is missing", parse(proto_type_t::setRollingCode, R"({"s":6})"));
+  TEST_ASSERT_EQUAL_STRING("lastRollingCode must be 0-65535", parse(proto_type_t::setRollingCode, R"({"s":6,"c":65536})"));
+}
+static void test_a_type_that_is_not_a_shade_op() {
+  TEST_ASSERT_EQUAL_STRING("Not a shade op", parse(proto_type_t::sendRemoteCommand, R"({})"));
+}
+
+/*********************************************************************
+ * A frame as a remote
+ ********************************************************************/
+static void test_remote_command() {
+  doc.clear();
+  deserializeJson(doc, R"({"a":1193046,"cm":"Up","c":42,"ek":160,"rp":3})");
+  somfy_frame_t frame;
+  uint8_t repeats = 9;
+  TEST_ASSERT_GOOD(parseRemoteCommandOp(doc.as<JsonObjectConst>(), frame, repeats));
+  TEST_ASSERT_EQUAL_UINT32(0x123456, frame.remoteAddress);
+  TEST_ASSERT_EQUAL(somfy_commands::Up, frame.cmd);
+  TEST_ASSERT_EQUAL_UINT16(42, frame.rollingCode);
+  TEST_ASSERT_EQUAL_UINT8(160, frame.encKey);
+  TEST_ASSERT_EQUAL_UINT8(3, repeats);
+}
+static void test_remote_command_defaults() {
+  doc.clear();
+  deserializeJson(doc, R"({"a":1,"cm":"My","c":1})");
+  somfy_frame_t frame;
+  uint8_t repeats = 9;
+  TEST_ASSERT_GOOD(parseRemoteCommandOp(doc.as<JsonObjectConst>(), frame, repeats));
+  TEST_ASSERT_EQUAL_UINT8(0xA7, frame.encKey);
+  TEST_ASSERT_EQUAL_UINT8(0, repeats);
+}
+static void test_remote_command_needs_an_address_and_a_rolling_code() {
+  somfy_frame_t frame;
+  uint8_t repeats;
+  doc.clear();
+  deserializeJson(doc, R"({"cm":"Up","c":1})");
+  TEST_ASSERT_EQUAL_STRING("remoteAddress is missing", parseRemoteCommandOp(doc.as<JsonObjectConst>(), frame, repeats));
+  doc.clear();
+  deserializeJson(doc, R"({"a":0,"cm":"Up","c":1})");
+  TEST_ASSERT_EQUAL_STRING("remoteAddress must be 1-16777215", parseRemoteCommandOp(doc.as<JsonObjectConst>(), frame, repeats));
+  doc.clear();
+  deserializeJson(doc, R"({"a":5,"cm":"Up","c":0})");
+  TEST_ASSERT_EQUAL_STRING("lastRollingCode must be 1-65535", parseRemoteCommandOp(doc.as<JsonObjectConst>(), frame, repeats));
+}
+
+/*********************************************************************
+ * Replies once loop() has run a request
+ ********************************************************************/
+static void test_request_results_become_replies() {
+  shade_request_t shade;
+  shade_request_t group;
+  group.type = shade_request_type_t::group_sensor;
+  TEST_ASSERT_EQUAL(proto_error_t::none, protoRequestError(shade_request_result_t::done));
+  TEST_ASSERT_NULL(protoRequestErrorDesc(shade, shade_request_result_t::done));
+  TEST_ASSERT_EQUAL(proto_error_t::notFound, protoRequestError(shade_request_result_t::not_found));
+  TEST_ASSERT_EQUAL_STRING("No shade with that id", protoRequestErrorDesc(shade, shade_request_result_t::not_found));
+  TEST_ASSERT_EQUAL_STRING("No group with that id", protoRequestErrorDesc(group, shade_request_result_t::not_found));
+  TEST_ASSERT_EQUAL(proto_error_t::radioUnavailable, protoRequestError(shade_request_result_t::radio_refused));
+  TEST_ASSERT_EQUAL_STRING("The radio didn't take the command", protoRequestErrorDesc(shade, shade_request_result_t::radio_refused));
+  // A favorite waits for the shade to stop: the client may try again.
+  TEST_ASSERT_EQUAL(proto_error_t::busy, protoRequestError(shade_request_result_t::shade_moving));
+  TEST_ASSERT_EQUAL_STRING("The shade is moving", protoRequestErrorDesc(shade, shade_request_result_t::shade_moving));
+  TEST_ASSERT_EQUAL(proto_error_t::badRequest, protoRequestError(shade_request_result_t::no_tilt));
+  TEST_ASSERT_EQUAL_STRING("The shade has no tilt", protoRequestErrorDesc(shade, shade_request_result_t::no_tilt));
+}
+
+int main(int argc, char **argv) {
+  (void)argc;
+  (void)argv;
+  UNITY_BEGIN();
+  RUN_TEST(test_every_command_op_but_the_remote_frame_is_a_shade_request);
+  RUN_TEST(test_shade_command_by_name);
+  RUN_TEST(test_shade_command_to_a_target);
+  RUN_TEST(test_shade_command_with_repeat_and_step);
+  RUN_TEST(test_shade_command_needs_a_command_or_a_target_not_both);
+  RUN_TEST(test_shade_command_values_are_checked);
+  RUN_TEST(test_only_the_first_problem_is_reported);
+  RUN_TEST(test_a_left_out_payload_is_missing_everything);
+  RUN_TEST(test_tilt_command);
+  RUN_TEST(test_group_command_needs_a_command);
+  RUN_TEST(test_repeat_takes_a_shade_or_a_group);
+  RUN_TEST(test_command_names_read_back_as_shade_commanded_reports_them);
+  RUN_TEST(test_set_positions_leaves_out_what_was_not_given);
+  RUN_TEST(test_sensor_leaves_out_what_was_not_given);
+  RUN_TEST(test_favorite);
+  RUN_TEST(test_unpair_and_rolling_code);
+  RUN_TEST(test_a_type_that_is_not_a_shade_op);
+  RUN_TEST(test_remote_command);
+  RUN_TEST(test_remote_command_defaults);
+  RUN_TEST(test_remote_command_needs_an_address_and_a_rolling_code);
+  RUN_TEST(test_request_results_become_replies);
+  return UNITY_END();
+}

test/test_shaderequests/test_shaderequests.cpp

@@ -100,19 +100,19 @@ void tearDown() {}
 /*********************************************************************
  * Finding the shade or group
  ********************************************************************/
-static void test_a_missing_shade_is_dropped() {
+static void test_a_missing_shade_is_not_found() {
   shade_request_t req = request(shade_request_type_t::shade_command);
   req.target = 30;
-  TEST_ASSERT_TRUE(requests.run(req));
+  TEST_ASSERT_EQUAL(shade_request_result_t::not_found, requests.run(req));
   TEST_ASSERT_EQUAL_STRING("", ops.calls.c_str());
 }
-static void test_a_missing_group_is_dropped() {
+static void test_a_missing_group_is_not_found() {
-  TEST_ASSERT_TRUE(requests.run(request(shade_request_type_t::group_command, 2)));
+  TEST_ASSERT_EQUAL(shade_request_result_t::not_found, requests.run(request(shade_request_type_t::group_command, 2)));
   TEST_ASSERT_EQUAL_STRING("", ops.calls.c_str());
 }
 static void test_a_shade_id_is_not_a_group_id() {
   ops.shades[2] = rollerShade();
-  requests.run(request(shade_request_type_t::group_command, 2));
+  TEST_ASSERT_EQUAL(shade_request_result_t::not_found, requests.run(request(shade_request_type_t::group_command, 2)));
   TEST_ASSERT_EQUAL_STRING("", ops.calls.c_str());
 }
 
@@ -174,12 +174,19 @@ static void test_set_positions_leaves_the_tilt_of_a_shade_without_one() {
   requests.run(req);
   TEST_ASSERT_EQUAL_STRING("setPositions 4 20 -1", ops.calls.c_str());
 }
+static void test_set_positions_tilt_alone_on_a_shade_without_one_is_refused() {
+  ops.shades[4] = rollerShade();
+  shade_request_t req = request(shade_request_type_t::set_positions);
+  req.tiltPosition = 40;
+  TEST_ASSERT_EQUAL(shade_request_result_t::no_tilt, requests.run(req));
+  TEST_ASSERT_EQUAL_STRING("", ops.calls.c_str());
+}
 static void test_set_positions_with_nothing_in_range_does_nothing() {
   ops.shades[4] = tiltShade(tilt_types::integrated);
   shade_request_t req = request(shade_request_type_t::set_positions);
   req.position = -1;
   req.tiltPosition = 101;
-  TEST_ASSERT_TRUE(requests.run(req));
+  TEST_ASSERT_EQUAL(shade_request_result_t::done, requests.run(req));
   TEST_ASSERT_EQUAL_STRING("", ops.calls.c_str());
 }
 static void test_client_values_are_checked_on_the_way_in() {
@@ -279,6 +286,21 @@ static void test_group_sensor_uses_the_groups_repeats() {
 /*********************************************************************
  * The favorite
  ********************************************************************/
+static void test_favorite_on_a_moving_shade_is_refused() {
+  ops.shades[4] = rollerShade();
+  ops.shades[4].idle = false;
+  shade_request_t req = request(shade_request_type_t::set_favorite);
+  req.position = 50;
+  TEST_ASSERT_EQUAL(shade_request_result_t::shade_moving, requests.run(req));
+  TEST_ASSERT_EQUAL_STRING("", ops.calls.c_str());
+}
+static void test_group_requests_take_a_group_id() {
+  TEST_ASSERT_TRUE(shadeRequestIsForGroup(shade_request_type_t::group_command));
+  TEST_ASSERT_TRUE(shadeRequestIsForGroup(shade_request_type_t::group_repeat));
+  TEST_ASSERT_TRUE(shadeRequestIsForGroup(shade_request_type_t::group_sensor));
+  TEST_ASSERT_FALSE(shadeRequestIsForGroup(shade_request_type_t::shade_sensor));
+  TEST_ASSERT_FALSE(shadeRequestIsForGroup(shade_request_type_t::unpair));
+}
 static void test_favorite_on_a_flipped_shade_is_turned_round() {
   ops.shades[4] = flippedShade();
   shade_request_t req = request(shade_request_type_t::set_favorite);
@@ -338,14 +360,14 @@ static void test_unpair_holds_prog_longer_on_a_56_bit_motor() {
 static void test_unpair_the_radio_refused_leaves_the_shade_paired() {
   ops.shades[4] = rollerShade();
   ops.radioTakes = false;
-  TEST_ASSERT_FALSE(requests.run(request(shade_request_type_t::unpair)));
+  TEST_ASSERT_EQUAL(shade_request_result_t::radio_refused, requests.run(request(shade_request_type_t::unpair)));
   TEST_ASSERT_EQUAL_STRING("sendCommand 4 Prog repeat=7 step=0", ops.calls.c_str());
 }
 static void test_rolling_code_is_set() {
   ops.shades[4] = rollerShade();
   shade_request_t req = request(shade_request_type_t::set_rolling_code);
   req.rollingCode = 1234;
-  TEST_ASSERT_TRUE(requests.run(req));
+  TEST_ASSERT_EQUAL(shade_request_result_t::done, requests.run(req));
   TEST_ASSERT_EQUAL_STRING("setRollingCode 4 1234", ops.calls.c_str());
 }
 
@@ -357,15 +379,15 @@ static void test_a_refused_command_is_reported() {
   ops.radioTakes = false;
   shade_request_t req = request(shade_request_type_t::shade_command);
   req.target = 50;
-  TEST_ASSERT_FALSE(requests.run(req));
+  TEST_ASSERT_EQUAL(shade_request_result_t::radio_refused, requests.run(req));
 }
 
 int main(int argc, char **argv) {
   (void)argc;
   (void)argv;
   UNITY_BEGIN();
-  RUN_TEST(test_a_missing_shade_is_dropped);
+  RUN_TEST(test_a_missing_shade_is_not_found);
-  RUN_TEST(test_a_missing_group_is_dropped);
+  RUN_TEST(test_a_missing_group_is_not_found);
   RUN_TEST(test_a_shade_id_is_not_a_group_id);
   RUN_TEST(test_target_moves_a_shade_there);
   RUN_TEST(test_target_on_a_flipped_shade_is_turned_round);
@@ -374,6 +396,7 @@ int main(int argc, char **argv) {
   RUN_TEST(test_tilt_without_a_target_sends_the_tilt_command);
   RUN_TEST(test_set_positions_on_a_flipped_shade_are_turned_round);
   RUN_TEST(test_set_positions_leaves_the_tilt_of_a_shade_without_one);
+  RUN_TEST(test_set_positions_tilt_alone_on_a_shade_without_one_is_refused);
   RUN_TEST(test_set_positions_with_nothing_in_range_does_nothing);
   RUN_TEST(test_client_values_are_checked_on_the_way_in);
   RUN_TEST(test_no_repeat_means_the_shades_own);
@@ -385,6 +408,8 @@ int main(int argc, char **argv) {
   RUN_TEST(test_group_repeat_repeats_the_last_frame);
   RUN_TEST(test_sensor_sends_what_was_given);
   RUN_TEST(test_group_sensor_uses_the_groups_repeats);
+  RUN_TEST(test_favorite_on_a_moving_shade_is_refused);
+  RUN_TEST(test_group_requests_take_a_group_id);
   RUN_TEST(test_favorite_on_a_flipped_shade_is_turned_round);
   RUN_TEST(test_favorite_without_a_tilt_keeps_the_favorite_tilt);
   RUN_TEST(test_favorite_tilt_alone_keeps_the_favorite_lift);