[hoermann_hcp] Send each command once, and the lamp as a state (#19765)

Co-authored-by: J. Nick Koston <nick@koston.org>

esphome/components/hoermann_hcp/hoermann_hcp.cpp

@@ -1,6 +1,7 @@
 #include "hoermann_hcp.h"
 
 #include <algorithm>
+#include <utility>
 
 #include "esphome/core/hal.h"
 #include "esphome/core/helpers.h"
@@ -16,20 +17,17 @@ static constexpr uint16_t STATE_REG = 0x9CB9;      // Internal state read back b
 static constexpr uint16_t BROADCAST_REG = 0x9D31;  // Door status broadcast by the bus controller
 static constexpr float CLOSE_POSITION_THRESHOLD = 0.05f;
 static constexpr float OPEN_POSITION_THRESHOLD = 0.95f;
-// Only the parity of the outstanding toggles says where the lamp is heading, so the count must not run away.
-static constexpr uint8_t MAX_LIGHT_TOGGLES_IN_FLIGHT = 4;
 
-// Command encoding: the high byte of the first register is the phase (0x02 pressed, 0x01 released) and the
-// rest names the button - the low byte for the door commands, the second register for those that do not fit
-// there. Both halves repeat that name, so neither register is a level to hold; they carry one event each.
-static constexpr HoermannHcpCommand COMMAND_OPEN{"open", 0x0210, 0x0110};
-static constexpr HoermannHcpCommand COMMAND_CLOSE{"close", 0x0220, 0x0120};
-static constexpr HoermannHcpCommand COMMAND_IMPULSE{"impulse", 0x0240, 0x0140};
-// The intermediate positions are named in the second register, so the first only carries the phase.
-static constexpr HoermannHcpCommand COMMAND_VENT{"vent", 0x0200, 0x0100, 0x4000, 0x4000};
-static constexpr HoermannHcpCommand COMMAND_HALF_OPEN{"half open", 0x0200, 0x0100, 0x0400, 0x0400};
-// The lamp is named in the second register, but its phase bytes follow no scheme the door commands share.
-static constexpr HoermannHcpCommand COMMAND_TOGGLE_LAMP{"toggle light", 0x0100, 0x0800, 0x0200, 0x0200, false};
+static constexpr HoermannHcpCommand COMMAND_OPEN{"open", 0x0110};
+static constexpr HoermannHcpCommand COMMAND_CLOSE{"close", 0x0120};
+static constexpr HoermannHcpCommand COMMAND_IMPULSE{"impulse", 0x0140};
+static constexpr HoermannHcpCommand COMMAND_VENT{"vent", 0x0100, 0x4000};
+static constexpr HoermannHcpCommand COMMAND_HALF_OPEN{"half open", 0x0100, 0x0400};
+// Absolute, as a vendor gateway sends them, so a late or repeated one cannot switch the lamp the wrong way.
+static constexpr HoermannHcpCommand COMMAND_LIGHT_ON{"light on", 0x0880};
+static constexpr HoermannHcpCommand COMMAND_LIGHT_OFF{"light off", 0x0800, 0x0100};
+// Kept as the intent, as the same impulse starts a door at rest; only a door still moving at the fetch gets it.
+static constexpr HoermannHcpCommand COMMAND_STOP{"stop", 0x0140};
 
 // High byte of the state register and the door state it stands for. State 0x00 is decoded separately because
 // its low byte tells a plain stop from the vent position.
@@ -63,9 +61,24 @@ static bool is_moving(DoorState state) {
   }
 }
 
-#ifdef USE_HOERMANN_HCP_IDENTITY
-// The command byte of a status poll. Only its answer can carry a request.
+static bool at_destination(const HoermannHcpCommand &command, DoorState state) {
+  if (&command == &COMMAND_OPEN)
+    return state == DoorState::OPEN;
+  if (&command == &COMMAND_CLOSE)
+    return state == DoorState::CLOSED;
+  if (&command == &COMMAND_VENT)
+    return state == DoorState::VENT;
+  if (&command == &COMMAND_HALF_OPEN)
+    return state == DoorState::HALF_OPEN;
+  return false;
+}
+
+// Only a status poll's answer carries commands.
 static constexpr uint8_t STATUS_COMMAND = 0x03;
+// A second stop this soon after one went out is ignored, so a double press cannot restart the door.
+static constexpr uint32_t STOP_LOCK_MS = 500;
+
+#ifdef USE_HOERMANN_HCP_IDENTITY
 // A status answer with this code in the low byte of its second register asks the bus controller for a value,
 // named in the high byte of the third.
 static constexpr uint8_t ANSWER_REQUEST = 0x22;
@@ -125,29 +138,40 @@ void HoermannHcp::update() {
     this->set_valid_(false);
   // Status broadcasts alone keep the connection alive, so a command the controller never fetches would
   // otherwise block every later one for as long as it keeps broadcasting.
-  if (this->next_command_ != nullptr && now - this->command_queued_at_ > this->connection_timeout_ms_) {
-    // Dropping after the press was presented leaves the door without its release value, which is worth saying
-    // apart from a command the controller never looked at.
-    if (this->command_written_at_ != 0) {
-      ESP_LOGW(TAG, "Bus controller stopped polling during '%s' command, dropping it mid key press",
-               this->next_command_->name);
-    } else {
-      ESP_LOGW(TAG, "Bus controller did not fetch '%s' command, dropping it", this->next_command_->name);
-    }
+  // A stop held for a door that has not reported its start yet is timed by the start window instead.
+  const bool stop_held = this->next_command_ == &COMMAND_STOP && this->starting_;
+  if (this->next_command_ != nullptr && !stop_held && now - this->command_queued_at_ > this->connection_timeout_ms_) {
+    ESP_LOGW(TAG, "Bus controller did not fetch '%s' command, dropping it", this->next_command_->name);
     this->drop_command_();
     // Children may have assumed the command would land, so let them re-derive from the door.
     this->changed_ = true;
   }
-  // A target waits for a door still travelling the other way to turn around. If it never does, the target has
-  // to go as well, otherwise it would cut a later move short. The connection timeout doubles as that window.
-  if (this->has_target_() && !this->target_started_ && now - this->target_queued_at_ > this->connection_timeout_ms_) {
+  // A target the door never started towards would otherwise cut a later move short.
+  if (this->has_target_() && !this->target_started_ && now - this->target_queued_at_ > this->start_window_ms_) {
     ESP_LOGW(TAG, "Door did not start moving towards the requested position, dropping it");
     this->clear_target_();
   }
-  // The door took the lamp key press but never reported the lamp changing, so stop expecting it to.
-  if (this->light_toggle_released_at_ != 0 && now - this->light_toggle_released_at_ > this->connection_timeout_ms_) {
-    ESP_LOGW(TAG, "Door did not report the lamp changing, giving up on the toggle");
-    this->forget_light_toggles_();
+  // A door that never answers a fetched command is at rest after all.
+  if (this->starting_ && now - this->start_fetched_at_ > this->start_window_ms_) {
+    this->starting_ = false;
+    if (stop_held) {
+      ESP_LOGW(TAG, "Door did not report moving, dropping the stop");
+      this->next_command_ = nullptr;
+    } else {
+      ESP_LOGD(TAG, "Door did not start after the command");
+    }
+  }
+  // A lamp command held while the door starts or moves waits on purpose, so its deadline starts once the door rests.
+  if (this->is_moving_or_starting_() && this->light_requested_ && !this->light_command_sent_)
+    this->light_since_ = now;
+  // Neither fire late nor block the next request.
+  if (this->light_requested_ && now - this->light_since_ > this->connection_timeout_ms_) {
+    if (this->light_command_sent_) {
+      ESP_LOGW(TAG, "Door did not report the lamp changing, giving up");
+    } else {
+      ESP_LOGW(TAG, "Bus controller did not fetch the lamp command, dropping it");
+    }
+    this->clear_light_request_();
   }
 #ifdef USE_HOERMANN_HCP_IDENTITY
   this->publish_identity_();
@@ -179,6 +203,7 @@ modbus::ResponseStatus HoermannHcp::on_read_holding_registers(uint16_t start_add
   }
 
   this->record_response_();
+  const bool status_poll = std::exchange(this->status_poll_pending_, false);
 
 #ifdef USE_HOERMANN_HCP_IDENTITY
   // Acknowledge the transfer taken by the write half of this frame.
@@ -198,7 +223,11 @@ modbus::ResponseStatus HoermannHcp::on_read_holding_registers(uint16_t start_add
       // Command request: return the internal state, injecting any pending command.
       registers.push_back(counter);
       registers.push_back(static_cast<uint16_t>(0x0001 | command));
-      this->push_command_registers_(registers);
+      if (status_poll && static_cast<uint8_t>(this->command_reg_value_) == STATUS_COMMAND) {
+        this->push_command_registers_(registers);
+      } else {
+        push_zeros(registers, 2);
+      }
       push_zeros(registers, 4);
 #ifdef USE_HOERMANN_HCP_IDENTITY
       this->add_identity_request_(registers, command);
@@ -234,6 +263,7 @@ modbus::ResponseStatus HoermannHcp::on_write_registers(uint16_t start_address,
     // command byte back from STATE_REG. The hub always runs the write before the read within one request.
     this->record_response_();
     this->command_reg_value_ = registers[0];
+    this->status_poll_pending_ = true;
 #ifdef USE_HOERMANN_HCP_IDENTITY
     this->transfer_answer_counter_ = this->take_identity_transfer_(registers);
 #endif
@@ -268,34 +298,62 @@ modbus::ResponseStatus HoermannHcp::on_write_registers(uint16_t start_address,
 }
 
 void HoermannHcp::push_command_registers_(modbus::RegisterValues &registers) {
-  const HoermannHcpCommand *command = this->next_command_;
+  const HoermannHcpCommand *command = this->take_command_();
+  if (command == nullptr)
+    command = this->take_light_command_();
   if (command == nullptr) {
     push_zeros(registers, 2);
     return;
   }
-  if (this->command_written_at_ == 0) {
-    // First read after the command was queued: present the "key pressed" values.
-    this->command_written_at_ = millis();
-    ESP_LOGI(TAG, "Sending '%s' command to door", command->name);
-    registers.push_back(command->pressed_value);
-    registers.push_back(command->pressed_value_2);
-    return;
-  }
-  if (millis() - this->command_written_at_ <= this->key_press_delay_ms_) {
-    // Between the two events there is nothing to report, including in the second register.
-    push_zeros(registers, 2);
-    return;
-  }
-  // Enough time passed: present the "key released" values and clear the command.
-  ESP_LOGD(TAG, "Released '%s' command", command->name);
-  this->command_written_at_ = 0;
+  ESP_LOGI(TAG, "Sending '%s' command to door", command->name);
+  registers.push_back(command->value);
+  registers.push_back(command->value_2);
+}
+
+const HoermannHcpCommand *HoermannHcp::take_command_() {
+  const HoermannHcpCommand *command = this->next_command_;
+  if (command == nullptr)
+    return nullptr;
+  const bool moving = is_moving(this->door_state_);
+  // The door was just told to start and has not said so yet, so an impulse now could start it instead.
+  if (command == &COMMAND_STOP && this->starting_ && !moving)
+    return nullptr;
   this->next_command_ = nullptr;
-  // A toggle whose count was already settled, by a lamp change reported from the door's side, has nothing left
-  // to wait for, so it must not re-arm the watchdog.
-  if (command == &COMMAND_TOGGLE_LAMP && this->light_toggles_in_flight_ != 0)
-    this->light_toggle_released_at_ = millis();
-  registers.push_back(command->released_value);
-  registers.push_back(command->released_value_2);
+  if (moving) {
+    // The door may have been started from elsewhere since the command was queued.
+    if ((command == &COMMAND_OPEN && this->door_state_ == DoorState::OPENING) ||
+        (command == &COMMAND_CLOSE && this->door_state_ == DoorState::CLOSING)) {
+      ESP_LOGD(TAG, "Door is already moving that way, dropping '%s'", command->name);
+      return nullptr;
+    }
+    if (command != &COMMAND_STOP) {
+      ESP_LOGD(TAG, "Door is moving, stopping it instead of '%s'", command->name);
+    }
+    this->last_stop_at_ = millis();
+    this->stop_sent_ = true;
+    return &COMMAND_STOP;
+  }
+  if (command == &COMMAND_STOP) {
+    ESP_LOGD(TAG, "Door came to rest before the stop was fetched, dropping it");
+    return nullptr;
+  }
+  // Until the door answers, it still reads as at rest.
+  if (!this->door_state_seen_ || !at_destination(*command, this->door_state_)) {
+    this->starting_ = true;
+    this->start_command_ = command;
+    this->start_fetched_at_ = millis();
+  }
+  return command;
+}
+
+const HoermannHcpCommand *HoermannHcp::take_light_command_() {
+  // The motor ignores the lamp while its door moves and may switch it itself as it starts, so the lamp waits for rest.
+  if (!this->light_requested_ || this->light_command_sent_ || this->is_moving_or_starting_() ||
+      this->light_target_ == this->light_on_)
+    return nullptr;
+  this->light_command_sent_ = true;
+  this->light_since_ = millis();
+  return this->light_target_ ? &COMMAND_LIGHT_ON : &COMMAND_LIGHT_OFF;
 }
 
 #ifdef USE_HOERMANN_HCP_IDENTITY
@@ -308,8 +366,8 @@ void HoermannHcp::add_identity_request_(modbus::RegisterValues &registers, uint1
       this->arm_identity_request_(IdentityPhase::IDENTITY_PHASE_SERIAL);
     return;
   }
-  // Uses the registers of a key press, so it waits while one is pending.
-  if (this->next_command_ != nullptr || registers[2] != 0 || registers[3] != 0 || !this->take_identity_request_())
+  // Uses the command registers, so it waits while a command is pending.
+  if (registers[2] != 0 || registers[3] != 0 || !this->take_identity_request_())
     return;
   registers[1] = static_cast<uint16_t>(ANSWER_REQUEST | command);
   registers[2] = encode_uint16(this->identity_request_(), 0);
@@ -505,56 +563,57 @@ bool HoermannHcp::queue_command_(const HoermannHcpCommand &command) {
     ESP_LOGW(TAG, "Not connected to the bus controller, dropping '%s' command", command.name);
     return false;
   }
+  // A stop still waiting for a door that has come to rest would be dropped at the fetch anyway.
+  if (this->next_command_ == &COMMAND_STOP)
+    this->next_command_ = nullptr;
   if (this->next_command_ != nullptr) {
     ESP_LOGW(TAG, "Previous command not yet fetched by the bus controller");
     return false;
   }
   // A new command supersedes any half-open target the door was still travelling to.
-  if (command.clears_target)
-    this->clear_target_();
+  this->clear_target_();
   this->next_command_ = &command;
   this->command_queued_at_ = millis();
   return true;
 }
 
-bool HoermannHcp::open_door() { return this->queue_command_(COMMAND_OPEN); }
-bool HoermannHcp::close_door() { return this->queue_command_(COMMAND_CLOSE); }
-bool HoermannHcp::impulse_door() { return this->queue_command_(COMMAND_IMPULSE); }
-bool HoermannHcp::vent_door() { return this->queue_command_(COMMAND_VENT); }
-bool HoermannHcp::half_open_door() { return this->queue_command_(COMMAND_HALF_OPEN); }
-bool HoermannHcp::toggle_light() {
-  if (this->light_toggles_in_flight_ >= MAX_LIGHT_TOGGLES_IN_FLIGHT) {
-    ESP_LOGW(TAG, "Too many lamp toggles are still waiting to be confirmed, dropping this one");
+bool HoermannHcp::is_moving_or_starting_() const { return this->starting_ || is_moving(this->door_state_); }
+
+bool HoermannHcp::command_door_(const HoermannHcpCommand &command) {
+  // Only stopped, so it is never reversed at speed. take_command_() drops one queued before the door started the same
+  // way.
+  if (this->is_moving_or_starting_())
+    return this->stop_door();
+  return this->queue_command_(command);
+}
+
+bool HoermannHcp::open_door() { return this->command_door_(COMMAND_OPEN); }
+bool HoermannHcp::close_door() { return this->command_door_(COMMAND_CLOSE); }
+bool HoermannHcp::impulse_door() { return this->command_door_(COMMAND_IMPULSE); }
+bool HoermannHcp::vent_door() { return this->command_door_(COMMAND_VENT); }
+bool HoermannHcp::half_open_door() { return this->command_door_(COMMAND_HALF_OPEN); }
+bool HoermannHcp::stop_door() {
+  this->clear_target_();
+  // A stop outranks whatever is still waiting, so a door at rest does not start after the user pressed stop.
+  if (this->next_command_ != nullptr && this->next_command_ != &COMMAND_STOP) {
+    ESP_LOGD(TAG, "Stop cancels the unfetched '%s' command", this->next_command_->name);
+    this->next_command_ = nullptr;
+  }
+  if (!this->is_moving_or_starting_())
+    return true;
+  if (!this->valid_) {
+    ESP_LOGW(TAG, "Not connected to the bus controller, dropping 'stop' command");
     return false;
   }
-  if (!this->queue_command_(COMMAND_TOGGLE_LAMP))
-    return false;
-  this->light_toggles_in_flight_++;
-  return true;
-}
-bool HoermannHcp::is_light_toggle_pending_() const { return this->next_command_ == &COMMAND_TOGGLE_LAMP; }
-
-uint8_t HoermannHcp::unsent_light_toggles_() const {
-  return this->is_light_toggle_pending_() && this->command_written_at_ == 0 ? 1 : 0;
-}
-
-bool HoermannHcp::cancel_light_toggle() {
-  // Once the pressed value has been presented the key press is already on the wire, so only an untouched
-  // command can be withdrawn.
-  if (!this->is_light_toggle_pending_() || this->command_written_at_ != 0)
-    return false;
-  ESP_LOGD(TAG, "Cancelling '%s' command the controller had not fetched", this->next_command_->name);
-  this->drop_command_();
-  return true;
-}
-
-bool HoermannHcp::stop_door() {
-  if (!is_moving(this->door_state_)) {
-    this->clear_target_();
+  if (this->next_command_ == &COMMAND_STOP)
+    return true;
+  if (this->stop_sent_ && millis() - this->last_stop_at_ < STOP_LOCK_MS) {
+    ESP_LOGD(TAG, "Door already stopping, ignoring stop");
     return true;
   }
-  // On success queue_command_() clears the target; on refusal it stays armed so the next position retries.
-  return this->queue_command_(COMMAND_IMPULSE);
+  this->next_command_ = &COMMAND_STOP;
+  this->command_queued_at_ = millis();
+  return true;
 }
 
 bool HoermannHcp::set_position(float position) {
@@ -563,8 +622,8 @@ bool HoermannHcp::set_position(float position) {
     return this->close_door();
   if (position >= OPEN_POSITION_THRESHOLD)
     return this->open_door();
-  // Asking the door to travel to where it already is means stopping it.
-  if (position == this->current_position_)
+  // Asking the door to travel to where it already is, or anywhere while it moves, means stopping it.
+  if (position == this->current_position_ || this->is_moving_or_starting_())
     return this->stop_door();
 
   // The door itself has no notion of a target, so it is started in the right direction and stopped on the way.
@@ -574,8 +633,6 @@ bool HoermannHcp::set_position(float position) {
   this->target_position_ = position;
   this->target_queued_at_ = millis();
   this->target_direction_ = opening ? DoorState::OPENING : DoorState::CLOSING;
-  // A door already travelling that way is on its way; one moving the other way has to turn around first.
-  this->target_started_ = this->door_state_ == this->target_direction_;
   return true;
 }
 
@@ -596,9 +653,8 @@ void HoermannHcp::set_valid_(bool valid) {
   ESP_LOGW(TAG, "Bus controller connection lost (no request for %" PRIu32 "ms)", millis() - this->last_response_);
   // Drop what the controller never fetched, so it neither blocks later commands nor fires on reconnect.
   this->drop_command_();
-  // The door cannot be watched while the bus is quiet, so a target left armed would stop it long afterwards.
-  this->clear_target_();
-  this->forget_light_toggles_();
+  this->starting_ = false;
+  this->clear_light_request_();
   // The lamp can be switched at the door while the bus is quiet, so what was last read is no longer trusted.
   this->set_light_seen_(false);
   // The same holds for the door. The next broadcast is decoded even if it repeats the last one.
@@ -608,38 +664,8 @@ void HoermannHcp::set_valid_(bool valid) {
 }
 
 void HoermannHcp::drop_command_() {
-  const bool was_light_toggle = this->is_light_toggle_pending_();
-  // Cleared first so the settling below no longer counts this command among the toggles still to be sent.
   this->next_command_ = nullptr;
-  this->command_written_at_ = 0;
-  if (was_light_toggle) {
-    // A lamp toggle says nothing about where the door was going, so it leaves the target alone.
-    this->light_toggle_settled_();
-  } else {
-    this->clear_target_();
-  }
-}
-
-void HoermannHcp::light_toggle_settled_() {
-  if (this->light_toggles_in_flight_ == 0)
-    return;
-  this->light_toggles_in_flight_--;
-  // Only a toggle the door has been shown can still be confirmed, so unsent ones leave nothing to wait for.
-  if (this->light_toggles_in_flight_ == this->unsent_light_toggles_())
-    this->light_toggle_released_at_ = 0;
-  // The light was showing where the lamp was heading, so it has to be told to look again.
-  this->changed_ = true;
-}
-
-void HoermannHcp::forget_light_toggles_() {
-  // Nothing outstanding must always mean nothing to wait for, or the watchdog below would fire for ever.
-  this->light_toggle_released_at_ = 0;
-  // A toggle the door has not been shown yet is still going to fire, so it keeps counting.
-  const uint8_t unsent = this->unsent_light_toggles_();
-  if (this->light_toggles_in_flight_ == unsent)
-    return;
-  this->light_toggles_in_flight_ = unsent;
-  this->changed_ = true;
+  this->clear_target_();
 }
 
 void HoermannHcp::set_door_state_(DoorState state) {
@@ -648,10 +674,21 @@ void HoermannHcp::set_door_state_(DoorState state) {
     this->door_state_seen_ = true;
     this->changed_ = true;
   }
+  // Only moving or the command's destination answers it, even as a first report that changes nothing.
+  if (this->starting_ && (is_moving(state) || at_destination(*this->start_command_, state))) {
+    // A stop held for the start is due now, so its fetch deadline starts here.
+    if (this->next_command_ == &COMMAND_STOP)
+      this->command_queued_at_ = millis();
+    this->starting_ = false;
+  }
   if (this->door_state_ == state)
     return;
   this->door_state_ = state;
   this->changed_ = true;
+  if (!is_moving(state)) {
+    // A door at rest cannot be restarted by a second stop, as stop_door() sends nothing then.
+    this->stop_sent_ = false;
+  }
   this->update_current_position_();
   if (!this->has_target_())
     return;
@@ -688,19 +725,48 @@ void HoermannHcp::set_light_on_(bool on) {
     return;
   this->light_on_ = on;
   this->changed_ = true;
-  if (this->light_toggles_in_flight_ <= this->unsent_light_toggles_()) {
-    // The door has not been shown a toggle that could explain this, so the lamp was switched at the door.
+  if (!this->light_requested_) {
     ESP_LOGD(TAG, "Lamp %s at the door", ONOFF(on));
     return;
   }
-  // The door acted, so one of the toggles it has seen has arrived. Any others still count.
-  this->light_toggle_settled_();
+  if (on == this->light_target_) {
+    this->clear_light_request_();
+    return;
+  }
+  // Switched away from the target while the command was out: send again.
+  this->light_command_sent_ = false;
+  this->light_since_ = millis();
+}
+
+bool HoermannHcp::set_light(bool on) {
+  // A known lamp implies a live connection.
+  if (!this->light_seen_)
+    return false;
+  this->light_target_ = on;
+  const bool was_requested = this->light_requested_;
+  // A sent command may still switch it away.
+  this->light_requested_ = on != this->light_on_ || this->light_command_sent_;
+  // The deadline belongs to the request, so more taps cannot keep it alive.
+  if (!was_requested)
+    this->light_since_ = millis();
+  this->changed_ = true;
+  return true;
+}
+
+void HoermannHcp::clear_light_request_() {
+  if (!this->light_requested_)
+    return;
+  this->light_requested_ = false;
+  this->light_command_sent_ = false;
+  this->changed_ = true;
 }
 
 void HoermannHcp::set_light_seen_(bool seen) {
   if (this->light_seen_ == seen)
     return;
   this->light_seen_ = seen;
+  if (!seen)
+    this->clear_light_request_();
   // A resting door changes nothing else, so without this the light would never hear about it.
   this->changed_ = true;
 }

esphome/components/hoermann_hcp/hoermann_hcp.h

@@ -47,17 +47,11 @@ enum class IdentityPhase : uint8_t {
 };
 #endif
 
-// A HCP command is a simulated key press: the pressed value is presented to the bus controller, then after a
-// short delay the released value. Each half also carries a second register, which names the buttons that do
-// not fit into the first.
+// Sent once, in a single status answer, as Hoermann's own bus accessory does.
 struct HoermannHcpCommand {
   const char *name;
-  uint16_t pressed_value;
-  uint16_t released_value;
-  uint16_t pressed_value_2{0x0000};
-  uint16_t released_value_2{0x0000};
-  // A door command supersedes a half-open target; the lamp has no bearing on where the door is going.
-  bool clears_target{true};
+  uint16_t value;
+  uint16_t value_2{0x0000};
 };
 
 class HoermannHcp : public PollingComponent, public modbus::ModbusServerDevice {
@@ -92,7 +86,8 @@ class HoermannHcp : public PollingComponent, public modbus::ModbusServerDevice {
   bool half_open_door();
   bool stop_door();
   bool set_position(float position);
-  bool toggle_light();
+  // False while the door has not reported the lamp.
+  bool set_light(bool on);
 
   DoorState get_door_state() const { return this->door_state_; }
   // False until a broadcast has carried a state the door is known to report. Bus traffic alone makes the
@@ -104,29 +99,20 @@ class HoermannHcp : public PollingComponent, public modbus::ModbusServerDevice {
   // False until a broadcast has actually carried the lamp register. Bus traffic alone makes the connection
   // valid without saying anything about the lamp, so is_light_on() would still be its default.
   bool is_light_known() const { return this->light_seen_; }
-  // Where the lamp ends up once every toggle on its way has landed, each of which inverts it. Until then the
-  // lamp still reads as its old self, so this is what a request has to be judged against.
-  bool is_light_heading_on() const { return this->light_on_ != (this->light_toggles_in_flight_ % 2 != 0); }
-  // Drops a lamp toggle the controller has not started reading, so a reversing request cancels it outright
-  // instead of fighting it. Returns false if there is nothing to cancel.
-  bool cancel_light_toggle();
+  // The requested state while switching, else the reported one.
+  bool is_light_heading_on() const { return this->light_requested_ ? this->light_target_ : this->light_on_; }
 
  protected:
-  // True while a lamp toggle is queued but not yet fetched, so the lamp is about to invert.
-  bool is_light_toggle_pending_() const;
-  // Toggles the door has not been shown yet, which is at most the one still waiting in the command slot.
-  uint8_t unsent_light_toggles_() const;
   void record_response_();
+  bool command_door_(const HoermannHcpCommand &command);
   // Returns false when the bus controller has not fetched the previous command yet.
   bool queue_command_(const HoermannHcpCommand &command);
-  // Throws away the pending command, taking any armed target with it unless the command was the lamp toggle.
   void drop_command_();
-  // One outstanding toggle reached the lamp, was withdrawn, or was thrown away.
-  void light_toggle_settled_();
-  // Stops expecting the toggles the door has already been shown to reach the lamp.
-  void forget_light_toggles_();
-  // Appends the two key-press registers and advances the pending command's press/release state.
+  void clear_light_request_();
   void push_command_registers_(modbus::RegisterValues &registers);
+  // Decide at the fetch what goes into a status answer, against the door as it stands then.
+  const HoermannHcpCommand *take_command_();
+  const HoermannHcpCommand *take_light_command_();
   void on_position_reg_(uint16_t value);
   void on_state_reg_(uint16_t value);
   void on_light_reg_(uint16_t value);
@@ -151,6 +137,7 @@ class HoermannHcp : public PollingComponent, public modbus::ModbusServerDevice {
   // Recomputes the reported position from position_raw_ and the current door state.
   void update_current_position_();
   bool has_target_() const { return this->target_position_ != 0.0f; }
+  bool is_moving_or_starting_() const;
   void clear_target_();
   void set_light_on_(bool on);
   void set_light_seen_(bool seen);
@@ -161,21 +148,24 @@ class HoermannHcp : public PollingComponent, public modbus::ModbusServerDevice {
   // Position the door was told to travel to; 0.0 means no target is armed.
   float target_position_{0.0f};
 
-  // Pending command / key-press state machine.
   const HoermannHcpCommand *next_command_{nullptr};
+  const HoermannHcpCommand *start_command_{nullptr};
   uint32_t command_queued_at_{0};
   // Separate from command_queued_at_ so an unrelated command cannot extend the target's start deadline.
   uint32_t target_queued_at_{0};
-  uint32_t command_written_at_{0};
   uint32_t last_response_{0};
-  // When the door was last handed a lamp key press. It reports the lamp a moment later, so this bounds the
-  // wait. Queueing another toggle deliberately leaves it alone, so the one already sent keeps its deadline.
-  uint32_t light_toggle_released_at_{0};
+  // Start of the wait for the fetch, then for the report.
+  uint32_t light_since_{0};
+  uint32_t last_stop_at_{0};
+  uint32_t start_fetched_at_{0};
+  bool stop_sent_{false};
+  // A door command was fetched and the door has not answered it by moving or reaching its destination yet.
+  bool starting_{false};
 
-  // A command is "pressed" for this long before its end value is sent.
-  uint16_t key_press_delay_ms_{100};
   // Drop the "connected" flag if the bus controller has not polled us for this long.
   uint16_t connection_timeout_ms_{2000};
+  // A chosen margin for a door to report moving after a fetched command.
+  uint16_t start_window_ms_{5000};
   // The state starts on a value the bus controller never reports, so the first broadcast is decoded even when
   // it reads 0x0000.
   uint16_t prev_state_reg_{0xFFFF};
@@ -184,19 +174,22 @@ class HoermannHcp : public PollingComponent, public modbus::ModbusServerDevice {
   uint16_t command_reg_value_{0};
 
   DoorState door_state_{DoorState::CLOSED};
-  // Direction the door was started in for the current target. A target armed while the door is still travelling
-  // the other way must not be judged by the reported direction until the door has turned around.
+  // Direction the door was started in for the current target, judged only once the door reports moving that way.
   DoorState target_direction_{DoorState::STOPPED};
   // Position as reported by the bus controller, 0..200 across the full travel.
   uint8_t position_raw_{0};
-  uint8_t light_toggles_in_flight_{0};
   bool target_started_{false};
   bool valid_{false};
   bool changed_{false};
   bool light_on_{false};
   bool light_seen_{false};
+  bool light_requested_{false};
+  bool light_command_sent_{false};
+  bool light_target_{false};
   bool door_state_seen_{false};
   bool short_broadcast_logged_{false};
+  // Only the read half right after a 0x17 write carries a command, so a second read without a new write does not.
+  bool status_poll_pending_{false};
 
 #ifdef USE_HOERMANN_HCP_IDENTITY
   uint32_t identity_asked_at_{0};

esphome/components/hoermann_hcp/light/hoermann_hcp_light.cpp

@@ -37,15 +37,10 @@ void HoermannHcpLight::write_state(light::LightState *state) {
   if (restored) {
     ESP_LOGD(TAG, "Ignoring the restored state, the door decides what the lamp is doing");
   } else if (published != binary) {
-    if (!this->parent_->is_light_known()) {
-      // Commanding a lamp that has not been read could switch off one that is already on.
-      ESP_LOGW(TAG, "Door has not reported the lamp yet, ignoring the requested state");
-    } else if (this->parent_->cancel_light_toggle() || this->parent_->toggle_light()) {
-      // A toggle the controller has not fetched is withdrawn outright rather than fought with a second one.
+    // Refused until the door has reported the lamp, as commanding an unread one could switch off a lit lamp.
+    if (this->parent_->set_light(binary))
       return;
-    } else {
-      ESP_LOGW(TAG, "Light command was not accepted by the door");
-    }
+    ESP_LOGW(TAG, "Door has not reported the lamp yet, ignoring the requested state");
   }
   // Nothing was sent, so the entity has to go back to showing the lamp rather than the request.
   this->publish_lamp_state_(heading_on);

tests/components/hoermann_hcp/button/hoermann_hcp_button_test.cpp

@@ -6,7 +6,7 @@
 
 namespace esphome::hoermann_hcp::testing {
 
-// The intermediate positions are named in the second register, which repeats that name on release.
+// The intermediate positions are named in the second register, next to 0x0100 in the first.
 TEST(HoermannHcpButtonTest, VentButtonSendsTheVentCommand) {
   TestableHoermannHcp door;
   HoermannHcpVentButton vent(&door);
@@ -14,13 +14,13 @@ TEST(HoermannHcpButtonTest, VentButtonSendsTheVentCommand) {
 
   vent.press();
 
-  auto [pressed, pressed_2] = poll_command(door);
-  EXPECT_EQ(pressed, 0x0200);
-  EXPECT_EQ(pressed_2, 0x4000);
-  std::this_thread::sleep_for(KEY_PRESS_ELAPSED);
-  auto [released, released_2] = poll_command(door);
-  EXPECT_EQ(released, 0x0100);
-  EXPECT_EQ(released_2, 0x4000);
+  auto [value, value_2] = poll_command(door);
+  EXPECT_EQ(value, 0x0100);
+  EXPECT_EQ(value_2, 0x4000);
+  // Sent once, in the first register as well as in the second.
+  auto [after, after_2] = poll_command(door);
+  EXPECT_EQ(after, 0x0000);
+  EXPECT_EQ(after_2, 0x0000);
 }
 
 TEST(HoermannHcpButtonTest, HalfOpenButtonSendsTheHalfOpenCommand) {
@@ -30,13 +30,13 @@ TEST(HoermannHcpButtonTest, HalfOpenButtonSendsTheHalfOpenCommand) {
 
   half_open.press();
 
-  auto [pressed, pressed_2] = poll_command(door);
-  EXPECT_EQ(pressed, 0x0200);
-  EXPECT_EQ(pressed_2, 0x0400);
-  std::this_thread::sleep_for(KEY_PRESS_ELAPSED);
-  auto [released, released_2] = poll_command(door);
-  EXPECT_EQ(released, 0x0100);
-  EXPECT_EQ(released_2, 0x0400);
+  auto [value, value_2] = poll_command(door);
+  EXPECT_EQ(value, 0x0100);
+  EXPECT_EQ(value_2, 0x0400);
+  // Sent once, in the first register as well as in the second.
+  auto [after, after_2] = poll_command(door);
+  EXPECT_EQ(after, 0x0000);
+  EXPECT_EQ(after_2, 0x0000);
 }
 
 // The door drives to the vent position on its own, so a position the cover was still travelling to must not

tests/components/hoermann_hcp/common.h

@@ -1,7 +1,5 @@
 #pragma once
-#include <chrono>
 #include <initializer_list>
-#include <thread>
 #include <utility>
 #include <gtest/gtest.h>
 #include "esphome/components/hoermann_hcp/hoermann_hcp.h"
@@ -14,9 +12,11 @@ using modbus::RegisterValues;
 constexpr uint16_t COMMAND_REG = 0x9C41;
 constexpr uint16_t STATE_REG = 0x9CB9;
 constexpr uint16_t BROADCAST_REG = 0x9D31;
-
-// The tests shorten the key-press delay to zero, so the release only needs the millis() clock to tick on.
-constexpr auto KEY_PRESS_ELAPSED = std::chrono::milliseconds(2);
+// The lamp commands a status answer carries in its two command registers.
+constexpr uint16_t LIGHT_ON = 0x0880;
+constexpr uint16_t LIGHT_ON_2 = 0x0000;
+constexpr uint16_t LIGHT_OFF = 0x0800;
+constexpr uint16_t LIGHT_OFF_2 = 0x0100;
 
 inline RegisterValues make_registers(std::initializer_list<uint16_t> values) {
   RegisterValues registers;
@@ -35,16 +35,15 @@ inline void connect_controller(HoermannHcp &door) {
   door.on_write_registers(COMMAND_REG, make_registers({0x0000, 0x0000}));
 }
 
-// Runs one status poll (write 2 / read 8) and returns the whole answer. The bus controller writes its counter
-// with command 0x03 here; most tests do not care and pass zero.
-inline RegisterValues status_answer(HoermannHcp &door, uint16_t command_reg = 0x0000) {
+// Runs one status poll (write 2 / read 8) and returns the whole answer.
+inline RegisterValues status_answer(HoermannHcp &door, uint16_t command_reg = 0x0003) {
   door.on_write_registers(COMMAND_REG, make_registers({command_reg, 0x0000}));
   RegisterValues response;
   door.on_read_holding_registers(STATE_REG, 8, response);
   return response;
 }
 
-// Runs one command poll (write 2 / read 8) and returns both key-press registers.
+// Runs one status poll and returns both command registers.
 inline std::pair<uint16_t, uint16_t> poll_command(HoermannHcp &door) {
   const RegisterValues response = status_answer(door);
   EXPECT_EQ(response.size(), 8u);
@@ -53,29 +52,24 @@ inline std::pair<uint16_t, uint16_t> poll_command(HoermannHcp &door) {
   return {response[2], response[3]};
 }
 
-// Presents and then releases the queued command, leaving the slot free.
-inline void consume_command(HoermannHcp &door) {
-  poll_command(door);
-  std::this_thread::sleep_for(KEY_PRESS_ELAPSED);
-  poll_command(door);
-}
+// Lets the controller fetch the queued command, leaving the slot free.
+inline void consume_command(HoermannHcp &door) { poll_command(door); }
 
 // Exposes the internal timings and the connection bookkeeping, so no test has to wait out a real delay.
 class TestableHoermannHcp : public HoermannHcp {
  public:
-  TestableHoermannHcp() { this->key_press_delay_ms_ = 0; }
-
   using HoermannHcp::connection_timeout_ms_;
+  using HoermannHcp::start_window_ms_;
 #ifdef USE_HOERMANN_HCP_IDENTITY
   using HoermannHcp::identity_asked_at_;
   using HoermannHcp::identity_request_;
   using HoermannHcp::firmware_unreadable_;
   using HoermannHcp::serial_unreadable_;
 #endif
-  using HoermannHcp::is_light_toggle_pending_;
-  using HoermannHcp::key_press_delay_ms_;
-  using HoermannHcp::light_toggle_released_at_;
-  using HoermannHcp::light_toggles_in_flight_;
+  using HoermannHcp::last_stop_at_;
+  using HoermannHcp::light_requested_;
+  using HoermannHcp::light_since_;
+  using HoermannHcp::light_command_sent_;
   using HoermannHcp::set_valid_;
 };
 

tests/components/hoermann_hcp/cover/hoermann_hcp_cover_test.cpp

@@ -68,7 +68,7 @@ TEST(HoermannHcpCoverTest, OpenCommandOpensTheDoor) {
   connect_controller(door);
 
   cover.make_call().set_command_open().perform();
-  EXPECT_EQ(poll_command(door).first, 0x0210);  // COMMAND_OPEN pressed
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
 }
 
 // The same for cover.close, which arrives as a position of 0.0.
@@ -79,7 +79,7 @@ TEST(HoermannHcpCoverTest, CloseCommandClosesTheDoor) {
   connect_controller(door);
 
   cover.make_call().set_command_close().perform();
-  EXPECT_EQ(poll_command(door).first, 0x0220);  // COMMAND_CLOSE pressed
+  EXPECT_EQ(poll_command(door).first, 0x0120);  // COMMAND_CLOSE
 }
 
 TEST(HoermannHcpCoverTest, ToggleCommandSendsAnImpulse) {
@@ -89,7 +89,7 @@ TEST(HoermannHcpCoverTest, ToggleCommandSendsAnImpulse) {
   connect_controller(door);
 
   cover.make_call().set_command_toggle().perform();
-  EXPECT_EQ(poll_command(door).first, 0x0240);  // COMMAND_IMPULSE pressed
+  EXPECT_EQ(poll_command(door).first, 0x0140);  // COMMAND_IMPULSE
 }
 
 TEST(HoermannHcpCoverTest, StopCommandStopsAMovingDoor) {
@@ -101,7 +101,7 @@ TEST(HoermannHcpCoverTest, StopCommandStopsAMovingDoor) {
   door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0064, 0x0100}));
 
   cover.make_call().set_command_stop().perform();
-  EXPECT_EQ(poll_command(door).first, 0x0240);  // COMMAND_IMPULSE pressed
+  EXPECT_EQ(poll_command(door).first, 0x0140);  // COMMAND_IMPULSE
 }
 
 // A position between the end stops starts the door in the right direction; it is stopped there later.
@@ -112,7 +112,7 @@ TEST(HoermannHcpCoverTest, PositionCommandStartsTheDoorTowardsTheTarget) {
   connect_controller(door);
 
   cover.make_call().set_position(0.5f).perform();
-  EXPECT_EQ(poll_command(door).first, 0x0210);  // COMMAND_OPEN pressed
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
 }
 
 // A command the door cannot take is assumed to have worked by whoever sent it, so the unchanged state has

tests/components/hoermann_hcp/hoermann_hcp_test.cpp

@@ -33,30 +33,30 @@ TEST(HoermannHcpReadWrite, BusScanReturnsIdentification) {
   EXPECT_EQ(response[4], 0xa845);
 }
 
-// Without a queued command, the command poll (write 2 / read 8) reports idle and no key press.
+// Without a queued command, the command poll (write 2 / read 8) reports idle and no command.
 TEST(HoermannHcpReadWrite, IdleCommandPollHasNoCommand) {
   HoermannHcp door;
-  EXPECT_FALSE(door.on_write_registers(COMMAND_REG, make_registers({0x0000, 0x0000})).has_value());
+  EXPECT_FALSE(door.on_write_registers(COMMAND_REG, make_registers({0x0003, 0x0000})).has_value());
   RegisterValues response;
   auto status = door.on_read_holding_registers(STATE_REG, 8, response);
   EXPECT_FALSE(status.has_value());
   ASSERT_EQ(response.size(), 8u);
-  EXPECT_EQ(response[1], 0x0001);
+  EXPECT_EQ(response[1], 0x0301);
   EXPECT_EQ(response[2], 0x0000);
   EXPECT_EQ(response[3], 0x0000);
 }
 
-// A queued control command is injected into the next command poll as a simulated key press.
+// A queued control command is injected into the next command poll.
 TEST(HoermannHcpReadWrite, QueuedCommandIsInjectedIntoPoll) {
   HoermannHcp door;
   connect_controller(door);
   door.open_door();
-  EXPECT_FALSE(door.on_write_registers(COMMAND_REG, make_registers({0x0000, 0x0000})).has_value());
+  EXPECT_FALSE(door.on_write_registers(COMMAND_REG, make_registers({0x0003, 0x0000})).has_value());
   RegisterValues response;
   auto status = door.on_read_holding_registers(STATE_REG, 8, response);
   EXPECT_FALSE(status.has_value());
   ASSERT_EQ(response.size(), 8u);
-  EXPECT_EQ(response[2], 0x0210);  // COMMAND_OPEN "key pressed" value
+  EXPECT_EQ(response[2], 0x0110);  // COMMAND_OPEN
   EXPECT_EQ(response[3], 0x0000);
 }
 
@@ -68,20 +68,368 @@ TEST(HoermannHcpReadWrite, UnknownAddressIsRejected) {
   EXPECT_EQ(door.on_write_registers(0x1234, make_registers({0x0000})), modbus::ExceptionCode::ILLEGAL_DATA_ADDRESS);
 }
 
-// A command is held for the key-press duration, then released, and only then can the next one be queued.
-TEST(HoermannHcpReadWrite, CommandIsReleasedAfterTheKeyPressDelay) {
+// A door command goes out in one answer and frees the slot at once.
+TEST(HoermannHcpReadWrite, DoorCommandIsSentOnceAndFreesTheSlot) {
   TestableHoermannHcp door;
   connect_controller(door);
-  door.open_door();
-  EXPECT_EQ(poll_command(door).first, 0x0210);  // COMMAND_OPEN pressed
-  // Refused while one is pending: were it accepted, the release below would carry COMMAND_CLOSE's 0x0120.
-  door.close_door();
+  EXPECT_TRUE(door.open_door());
+  // Refused while one is unfetched.
+  EXPECT_FALSE(door.close_door());
 
-  std::this_thread::sleep_for(KEY_PRESS_ELAPSED);
-  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN released
-  // With the command gone, the next one is accepted again.
-  door.close_door();
-  EXPECT_EQ(poll_command(door).first, 0x0220);  // COMMAND_CLOSE pressed
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
+  EXPECT_EQ(poll_command(door).first, 0x0000);
+  // Once the door has run and come to rest, the slot takes the next command.
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0064, 0x0100}));
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x00C8, 0x2000}));
+  EXPECT_TRUE(door.close_door());
+  EXPECT_EQ(poll_command(door).first, 0x0120);  // COMMAND_CLOSE
+}
+
+// A moving door is only ever stopped, whatever it is asked to do.
+TEST(HoermannHcpReadWrite, MovingDoorIsOnlyStopped) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0100}));
+  ASSERT_EQ(door.get_door_state(), DoorState::OPENING);
+
+  EXPECT_TRUE(door.close_door());
+  EXPECT_EQ(poll_command(door).first, 0x0140);  // COMMAND_IMPULSE
+}
+
+// A second stop within 500 ms is the same press and must not restart the door.
+TEST(HoermannHcpReadWrite, SecondStopWithinHalfASecondIsIgnored) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0100}));
+  ASSERT_TRUE(door.stop_door());
+  EXPECT_EQ(poll_command(door).first, 0x0140);  // COMMAND_IMPULSE
+
+  // Still reported moving while it slows down.
+  EXPECT_TRUE(door.open_door());
+  EXPECT_EQ(poll_command(door).first, 0x0000);
+
+  door.last_stop_at_ -= 500;
+  EXPECT_TRUE(door.stop_door());
+  EXPECT_EQ(poll_command(door).first, 0x0140);
+}
+
+// A stop outranks a command still waiting, so a door at rest does not start after it.
+TEST(HoermannHcpReadWrite, StopCancelsAnUnfetchedCommand) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000}));
+  ASSERT_TRUE(door.open_door());
+  EXPECT_TRUE(door.stop_door());
+  EXPECT_EQ(poll_command(door).first, 0x0000);
+}
+
+// The impulse would start a door that came to rest while the stop waited, so the stop is dropped instead.
+TEST(HoermannHcpReadWrite, StopIsDroppedWhenTheDoorRestsBeforeTheFetch) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0100}));
+  ASSERT_TRUE(door.stop_door());
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x00C8, 0x2000}));
+  EXPECT_EQ(poll_command(door).first, 0x0000);
+}
+
+// Until the door reports the start it reads as at rest, so a command in between only stops it, once it moves.
+TEST(HoermannHcpReadWrite, CommandBeforeTheStartIsReportedBecomesAStop) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000}));
+  ASSERT_TRUE(door.open_door());
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
+
+  EXPECT_TRUE(door.close_door());
+  EXPECT_EQ(poll_command(door).first, 0x0000);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0004, 0x0100}));
+  EXPECT_EQ(poll_command(door).first, 0x0140);  // COMMAND_IMPULSE
+}
+
+// A door that never reports moving after its command is at rest after all, so later commands are its own.
+TEST(HoermannHcpReadWrite, StartWindowClosesWhenTheDoorNeverMoves) {
+  TestableHoermannHcp door;
+  door.start_window_ms_ = 20;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000}));
+  ASSERT_TRUE(door.open_door());
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
+  std::this_thread::sleep_for(std::chrono::milliseconds(30));
+  connect_controller(door);
+  door.update();
+
+  EXPECT_TRUE(door.open_door());
+  EXPECT_EQ(poll_command(door).first, 0x0110);
+}
+
+// Only the read half of a status poll carries a command, so a second read without a new write gets none.
+TEST(HoermannHcpReadWrite, SecondReadWithoutAWriteCarriesNoCommand) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000}));
+  door.on_write_registers(COMMAND_REG, make_registers({0x0003, 0x0000}));
+  RegisterValues first;
+  door.on_read_holding_registers(STATE_REG, 8, first);
+  ASSERT_TRUE(door.open_door());
+  RegisterValues second;
+  door.on_read_holding_registers(STATE_REG, 8, second);
+  ASSERT_EQ(second.size(), 8u);
+  EXPECT_EQ(second[2], 0x0000);
+  // The next status poll takes it.
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
+}
+
+// A command queued for a door at rest only stops it if the door was started from elsewhere before the fetch.
+TEST(HoermannHcpReadWrite, CommandForADoorStartedBeforeTheFetchStopsIt) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x00C8, 0x2000}));
+  ASSERT_TRUE(door.open_door());
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x00C0, 0x0200}));
+  EXPECT_EQ(poll_command(door).first, 0x0140);  // COMMAND_IMPULSE
+}
+
+// A door started from elsewhere the way the queued command wants keeps going, and the command is dropped.
+TEST(HoermannHcpReadWrite, CommandForADoorAlreadyMovingThatWayIsDropped) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000}));
+  ASSERT_TRUE(door.open_door());
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0004, 0x0100}));
+  EXPECT_EQ(poll_command(door).first, 0x0000);
+}
+
+// The same holds for a close queued while the door was open and then started closing from elsewhere.
+TEST(HoermannHcpReadWrite, CloseForADoorAlreadyClosingIsDropped) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x00C8, 0x2000}));
+  ASSERT_TRUE(door.close_door());
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x00C0, 0x0200}));
+  EXPECT_EQ(poll_command(door).first, 0x0000);
+}
+
+// Another rest state after a fetched start is not the door's answer yet, so the start window stays open.
+TEST(HoermannHcpReadWrite, RestStateAfterAStartKeepsTheWindow) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0010, 0x0061}));
+  ASSERT_EQ(door.get_door_state(), DoorState::VENT);
+  ASSERT_TRUE(door.open_door());
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0010, 0x0000}));
+  ASSERT_EQ(door.get_door_state(), DoorState::STOPPED);
+  // Still starting, so a close is only a stop, held until the door moves.
+  ASSERT_TRUE(door.close_door());
+  EXPECT_EQ(poll_command(door).first, 0x0000);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0014, 0x0100}));
+  EXPECT_EQ(poll_command(door).first, 0x0140);  // COMMAND_IMPULSE
+}
+
+// A first report showing the door where the command sent it ends the start window, even without a change.
+TEST(HoermannHcpReadWrite, FirstReportAtTheDestinationEndsTheStart) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  ASSERT_TRUE(door.close_door());
+  EXPECT_EQ(poll_command(door).first, 0x0120);  // COMMAND_CLOSE
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000}));
+  ASSERT_EQ(door.get_door_state(), DoorState::CLOSED);
+  ASSERT_TRUE(door.open_door());
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
+}
+
+// A stop held for a late start report is sent once the door reports moving.
+TEST(HoermannHcpReadWrite, HeldStopSurvivesALateStart) {
+  TestableHoermannHcp door;
+  door.connection_timeout_ms_ = 20;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000}));
+  ASSERT_TRUE(door.open_door());
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
+  ASSERT_TRUE(door.stop_door());
+  std::this_thread::sleep_for(std::chrono::milliseconds(30));
+  connect_controller(door);
+  door.update();
+
+  // The start is reported after the stop's own fetch deadline, which then starts over.
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0004, 0x0100}));
+  door.update();
+  EXPECT_EQ(poll_command(door).first, 0x0140);  // COMMAND_IMPULSE
+}
+
+// A held stop goes when the door never reports its start, and never becomes an impulse.
+TEST(HoermannHcpReadWrite, HeldStopIsDroppedWhenTheDoorNeverStarts) {
+  TestableHoermannHcp door;
+  door.start_window_ms_ = 20;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000}));
+  ASSERT_TRUE(door.open_door());
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
+  ASSERT_TRUE(door.stop_door());
+  std::this_thread::sleep_for(std::chrono::milliseconds(30));
+  connect_controller(door);
+  door.update();
+  EXPECT_EQ(poll_command(door).first, 0x0000);
+}
+
+// A command for where the door already rests does not move it, so the next command is its own.
+TEST(HoermannHcpReadWrite, CommandForTheCurrentEndOpensNoStartWindow) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x00C8, 0x2000}));
+  ASSERT_TRUE(door.open_door());
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
+  ASSERT_TRUE(door.close_door());
+  EXPECT_EQ(poll_command(door).first, 0x0120);  // COMMAND_CLOSE
+}
+
+// The stop lock ends with the door at rest, so a target reached soon after the next start still stops it.
+TEST(HoermannHcpReadWrite, StopLockEndsWhenTheDoorRests) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0100}));
+  ASSERT_TRUE(door.stop_door());
+  EXPECT_EQ(poll_command(door).first, 0x0140);  // COMMAND_IMPULSE
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0000}));
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003E, 0x0100}));
+  ASSERT_TRUE(door.stop_door());
+  EXPECT_EQ(poll_command(door).first, 0x0140);
+}
+
+// A stop pressed while disconnected is not kept for the reconnect, where it could start a door at rest.
+TEST(HoermannHcpReadWrite, StopWhileDisconnectedIsNotQueued) {
+  TestableHoermannHcp door;
+  door.connection_timeout_ms_ = 20;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0100}));
+  std::this_thread::sleep_for(std::chrono::milliseconds(30));
+  door.update();
+  ASSERT_FALSE(door.is_valid());
+  ASSERT_EQ(door.get_door_state(), DoorState::OPENING);  // the stale report the stop would be judged by
+  EXPECT_FALSE(door.stop_door());
+  connect_controller(door);
+  EXPECT_EQ(poll_command(door).first, 0x0000);
+}
+
+// Pressing stop twice before the fetch sends one impulse.
+TEST(HoermannHcpReadWrite, SecondStopBeforeTheFetchSendsOneImpulse) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0100}));
+  ASSERT_TRUE(door.stop_door());
+  ASSERT_TRUE(door.stop_door());
+  EXPECT_EQ(poll_command(door).first, 0x0140);  // COMMAND_IMPULSE
+  EXPECT_EQ(poll_command(door).first, 0x0000);
+}
+
+// A position asked for before the start is reported stops the door once it moves.
+TEST(HoermannHcpReadWrite, PositionBeforeTheStartIsReportedIsAStop) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000}));
+  ASSERT_TRUE(door.open_door());
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
+  ASSERT_TRUE(door.set_position(0.5f));
+  EXPECT_EQ(poll_command(door).first, 0x0000);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0004, 0x0100}));
+  EXPECT_EQ(poll_command(door).first, 0x0140);  // COMMAND_IMPULSE
+}
+
+// A stop left waiting for a door that came to rest does not block the next command.
+TEST(HoermannHcpReadWrite, StaleStopDoesNotBlockTheNextCommand) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x00C0, 0x0100}));
+  ASSERT_TRUE(door.stop_door());
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x00C8, 0x2000}));
+  EXPECT_TRUE(door.close_door());
+  EXPECT_EQ(poll_command(door).first, 0x0120);  // COMMAND_CLOSE
+}
+
+// Nor does it block a move to a position.
+TEST(HoermannHcpReadWrite, StaleStopDoesNotBlockAPosition) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x00C0, 0x0100}));
+  ASSERT_TRUE(door.stop_door());
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x00C8, 0x2000}));
+  EXPECT_TRUE(door.set_position(0.5f));
+  EXPECT_EQ(poll_command(door).first, 0x0120);  // COMMAND_CLOSE
+}
+
+// Close on a closed door and vent at the vent position are no moves either, but half open at vent is.
+TEST(HoermannHcpReadWrite, EveryEndOpensNoStartWindow) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000}));
+  ASSERT_TRUE(door.close_door());
+  EXPECT_EQ(poll_command(door).first, 0x0120);  // COMMAND_CLOSE
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0010, 0x0A00}));
+  ASSERT_EQ(door.get_door_state(), DoorState::VENT);
+  ASSERT_TRUE(door.vent_door());
+  EXPECT_EQ(poll_command(door).first, 0x0100);  // COMMAND_VENT
+  ASSERT_TRUE(door.half_open_door());
+  EXPECT_EQ(poll_command(door).first, 0x0100);  // COMMAND_HALF_OPEN
+  // Half open from vent is a move, so a stop now waits for its start.
+  ASSERT_TRUE(door.stop_door());
+  EXPECT_EQ(poll_command(door).first, 0x0000);
+}
+
+// Until the door has reported where it is, no command counts as a no-op.
+TEST(HoermannHcpReadWrite, CommandBeforeTheFirstReportOpensAStartWindow) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  ASSERT_TRUE(door.close_door());
+  EXPECT_EQ(poll_command(door).first, 0x0120);  // COMMAND_CLOSE
+  ASSERT_TRUE(door.stop_door());
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0064, 0x0200}));
+  EXPECT_EQ(poll_command(door).first, 0x0140);  // COMMAND_IMPULSE
+}
+
+// A stop dropped with the start window does not come back when the door moves after all.
+TEST(HoermannHcpReadWrite, DroppedHeldStopDoesNotFireLater) {
+  TestableHoermannHcp door;
+  door.start_window_ms_ = 20;
+  connect_controller(door);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000}));
+  ASSERT_TRUE(door.open_door());
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
+  ASSERT_TRUE(door.stop_door());
+  std::this_thread::sleep_for(std::chrono::milliseconds(30));
+  connect_controller(door);
+  door.update();
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0004, 0x0100}));
+  EXPECT_EQ(poll_command(door).first, 0x0000);
+}
+
+// A start does not hold off the stop that follows it.
+TEST(HoermannHcpReadWrite, StopRightAfterAStartIsSent) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  ASSERT_TRUE(door.impulse_door());
+  EXPECT_EQ(poll_command(door).first, 0x0140);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0100}));
+
+  EXPECT_TRUE(door.stop_door());
+  EXPECT_EQ(poll_command(door).first, 0x0140);
+}
+
+// Only a status poll (command 0x03) fetches a command; another 8-register read carries zeros.
+TEST(HoermannHcpReadWrite, CommandWaitsForAStatusPoll) {
+  TestableHoermannHcp door;
+  connect_controller(door);
+  ASSERT_TRUE(door.open_door());
+
+  // A transfer write (command 0x04) read back as 8 registers.
+  door.on_write_registers(COMMAND_REG, make_registers({0x0504, 0x0000}));
+  RegisterValues other;
+  door.on_read_holding_registers(STATE_REG, 8, other);
+  ASSERT_EQ(other.size(), 8u);
+  EXPECT_EQ(other[2], 0x0000);
+  EXPECT_EQ(other[3], 0x0000);
+
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
 }
 
 // Commands issued while the bus controller is absent are dropped instead of firing when it returns.
@@ -106,7 +454,7 @@ TEST(HoermannHcpReadWrite, ConnectionLossDropsThePendingCommand) {
   EXPECT_EQ(poll_command(door).first, 0x0000);
   // And the slot is free, so a new command is accepted.
   door.close_door();
-  EXPECT_EQ(poll_command(door).first, 0x0220);
+  EXPECT_EQ(poll_command(door).first, 0x0120);
 }
 
 // The connection is dropped by update() once the controller stops polling, which is what releases a
@@ -141,13 +489,13 @@ TEST(HoermannHcpReadWrite, UnfetchedCommandExpiresWhileConnected) {
 
   std::this_thread::sleep_for(std::chrono::milliseconds(220));
   // A status broadcast refreshes the connection without ever fetching the command.
-  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0064, 0x0100}));
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0064, 0x0000}));
   door.update();
   ASSERT_TRUE(door.is_valid());
 
   // With the stale command gone, the door accepts commands again.
   door.close_door();
-  EXPECT_EQ(poll_command(door).first, 0x0220);
+  EXPECT_EQ(poll_command(door).first, 0x0120);
 }
 
 // The 0x17 read half echoes the message counter and command byte written to COMMAND_REG, packed
@@ -232,10 +580,7 @@ TEST(HoermannHcpPosition, NearlyClosedTargetClosesTheDoor) {
   HoermannHcp door;
   connect_controller(door);
   door.set_position(0.02f);
-  RegisterValues response;
-  door.on_read_holding_registers(STATE_REG, 8, response);
-  ASSERT_EQ(response.size(), 8u);
-  EXPECT_EQ(response[2], 0x0220);  // COMMAND_CLOSE "key pressed" value
+  EXPECT_EQ(poll_command(door).first, 0x0120);  // COMMAND_CLOSE
 }
 
 // A half-open target starts the door moving towards the requested position.
@@ -243,10 +588,7 @@ TEST(HoermannHcpPosition, HalfOpenTargetOpensTheDoor) {
   HoermannHcp door;  // starts out fully closed
   connect_controller(door);
   door.set_position(0.5f);
-  RegisterValues response;
-  door.on_read_holding_registers(STATE_REG, 8, response);
-  ASSERT_EQ(response.size(), 8u);
-  EXPECT_EQ(response[2], 0x0210);  // COMMAND_OPEN "key pressed" value
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
 }
 
 // The door has no notion of a target, so it is stopped with an impulse once it travels past the request.
@@ -254,9 +596,7 @@ TEST(HoermannHcpPosition, TargetPositionStopsTheDoor) {
   TestableHoermannHcp door;
   connect_controller(door);
   door.set_position(0.5f);
-  EXPECT_EQ(poll_command(door).first, 0x0210);  // COMMAND_OPEN pressed
-  std::this_thread::sleep_for(KEY_PRESS_ELAPSED);
-  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN released
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
 
   // Position 20/200 = 0.1 while opening: short of the target, so the door keeps going.
   door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0014, 0x0100}));
@@ -265,7 +605,7 @@ TEST(HoermannHcpPosition, TargetPositionStopsTheDoor) {
 
   // Position 120/200 = 0.6 is past the target, so the door is stopped.
   door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0078, 0x0100}));
-  EXPECT_EQ(poll_command(door).first, 0x0240);  // COMMAND_IMPULSE pressed
+  EXPECT_EQ(poll_command(door).first, 0x0140);  // COMMAND_IMPULSE
 }
 
 // An impulse restarts a stopped door, so a frame reporting the stop and the target crossing at once
@@ -274,8 +614,6 @@ TEST(HoermannHcpPosition, StopReportedWithTheCrossingSendsNoImpulse) {
   TestableHoermannHcp door;
   connect_controller(door);
   door.set_position(0.5f);
-  EXPECT_EQ(poll_command(door).first, 0x0210);
-  std::this_thread::sleep_for(KEY_PRESS_ELAPSED);
   EXPECT_EQ(poll_command(door).first, 0x0110);
 
   door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0014, 0x0100}));
@@ -292,8 +630,6 @@ TEST(HoermannHcpPosition, TargetIsDroppedWhenTheDoorStopsShort) {
   TestableHoermannHcp door;
   connect_controller(door);
   door.set_position(0.5f);
-  EXPECT_EQ(poll_command(door).first, 0x0210);
-  std::this_thread::sleep_for(KEY_PRESS_ELAPSED);
   EXPECT_EQ(poll_command(door).first, 0x0110);
 
   // The door is stopped at 0.3 by a wall button, short of the requested 0.5.
@@ -307,77 +643,53 @@ TEST(HoermannHcpPosition, TargetIsDroppedWhenTheDoorStopsShort) {
   EXPECT_EQ(poll_command(door).first, 0x0000);
 }
 
-// A target armed while the door is still travelling the other way must not be judged by that old direction,
-// otherwise the very next position it reports counts as reached and stops the door where it stands.
-TEST(HoermannHcpPosition, TargetArmedWhileMovingTheOtherWayWaitsForTheTurnaround) {
+// A new position while the door moves only stops it.
+TEST(HoermannHcpPosition, NewPositionWhileMovingStopsTheDoor) {
   TestableHoermannHcp door;
   connect_controller(door);
-  // The door is closing, passing 60/200 = 0.3.
   door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0200}));
   ASSERT_EQ(door.get_door_state(), DoorState::CLOSING);
 
-  door.set_position(0.5f);
-  EXPECT_EQ(poll_command(door).first, 0x0210);  // COMMAND_OPEN pressed
-  std::this_thread::sleep_for(KEY_PRESS_ELAPSED);
-  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN released
-
-  // Still closing at 58/200 = 0.29: below the target, but not on the way to it.
-  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003A, 0x0200}));
+  EXPECT_TRUE(door.set_position(0.5f));
+  EXPECT_EQ(poll_command(door).first, 0x0140);  // COMMAND_IMPULSE
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0064, 0x0100}));
   EXPECT_EQ(poll_command(door).first, 0x0000);
-
-  // Now opening at 62/200 = 0.31, still short of the target.
-  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003E, 0x0100}));
-  EXPECT_EQ(poll_command(door).first, 0x0000);
-
-  // Past the target at 110/200 = 0.55, so the door is stopped.
-  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x006E, 0x0100}));
-  EXPECT_EQ(poll_command(door).first, 0x0240);  // COMMAND_IMPULSE pressed
 }
 
-// A motor turning around can report a momentary stop; dropping the target there would let the door run on
-// to the end stop that the reversing command asked for.
-TEST(HoermannHcpPosition, MomentaryStopWhileTurningAroundKeepsTheTarget) {
+// A target outlives a start reported late, as long as it comes within the start window.
+TEST(HoermannHcpPosition, TargetSurvivesALateStart) {
   TestableHoermannHcp door;
+  door.connection_timeout_ms_ = 20;
   connect_controller(door);
-  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0200}));
-  ASSERT_EQ(door.get_door_state(), DoorState::CLOSING);
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0000}));
+  ASSERT_TRUE(door.set_position(0.5f));
+  EXPECT_EQ(poll_command(door).first, 0x0110);  // COMMAND_OPEN
+  std::this_thread::sleep_for(std::chrono::milliseconds(30));
+  connect_controller(door);
+  door.update();
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003E, 0x0100}));
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0064, 0x0100}));
+  EXPECT_EQ(poll_command(door).first, 0x0140);  // COMMAND_IMPULSE
+}
 
-  door.set_position(0.5f);
-  EXPECT_EQ(poll_command(door).first, 0x0210);
-  std::this_thread::sleep_for(KEY_PRESS_ELAPSED);
-  EXPECT_EQ(poll_command(door).first, 0x0110);
-
-  // The stop reported on the way from closing to opening.
+// A door that never starts has to lose the target, otherwise it would cut a later move short.
+TEST(HoermannHcpPosition, TargetIsDroppedWhenTheDoorNeverStarts) {
+  TestableHoermannHcp door;
+  door.connection_timeout_ms_ = 200;
+  door.start_window_ms_ = 200;
+  connect_controller(door);
   door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0000}));
   ASSERT_EQ(door.get_door_state(), DoorState::STOPPED);
 
-  // The door then opens and still has to be stopped at the requested position.
-  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003E, 0x0100}));
-  EXPECT_EQ(poll_command(door).first, 0x0000);
-  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x006E, 0x0100}));
-  EXPECT_EQ(poll_command(door).first, 0x0240);
-}
-
-// A door that never turns around has to lose the target as well, otherwise it would cut a later move short.
-TEST(HoermannHcpPosition, TargetIsDroppedWhenTheDoorNeverTurnsAround) {
-  TestableHoermannHcp door;
-  door.connection_timeout_ms_ = 200;
-  connect_controller(door);
-  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0200}));
-  ASSERT_EQ(door.get_door_state(), DoorState::CLOSING);
-
   door.set_position(0.5f);
-  EXPECT_EQ(poll_command(door).first, 0x0210);
-  std::this_thread::sleep_for(KEY_PRESS_ELAPSED);
   EXPECT_EQ(poll_command(door).first, 0x0110);
 
   std::this_thread::sleep_for(std::chrono::milliseconds(220));
-  // The door ignored the command and closed all the way. Its broadcast keeps the connection alive, so the
-  // target is the only thing that may expire here.
-  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000}));
+  // The door ignored the command. Its broadcast keeps the connection alive, so the target is the only thing
+  // that may expire here.
+  door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0000}));
   door.update();
   ASSERT_TRUE(door.is_valid());
-  ASSERT_EQ(door.get_door_state(), DoorState::CLOSED);
 
   // A later manual open must run freely instead of being stopped at the abandoned target.
   door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003E, 0x0100}));

tests/components/hoermann_hcp/text_sensor/hoermann_hcp_text_sensor_test.cpp

@@ -2,7 +2,6 @@
 #include <gtest/gtest.h>
 
 #include <string>
-#include <thread>
 
 #include "esphome/components/text_sensor/text_sensor.h"
 
@@ -40,7 +39,7 @@ void write_transfer(HoermannHcp &door, uint8_t counter, uint8_t sub_code, const
 
 // A whole payload transfer, returning the answer the motor reads back.
 RegisterValues transfer(HoermannHcp &door, uint8_t counter, uint8_t sub_code, const char *bytes, size_t len,
-                        uint16_t read_registers = 8) {
+                        uint16_t read_registers = 2) {
   write_transfer(door, counter, sub_code, bytes, len);
   RegisterValues response;
   door.on_read_holding_registers(STATE_REG, read_registers, response);
@@ -100,8 +99,8 @@ TEST(HoermannHcpTextSensorTest, NothingChangesWithoutASensor) {
     EXPECT_EQ(response[1], 0x0301);
     EXPECT_EQ(response[2], 0x0000);
   }
-  const RegisterValues answer = transfer(door, FIRST_HALF | 0x05, SUB_SERIAL, SERIAL, 14);
-  EXPECT_EQ(answer[1] & 0x00FF, 0x0001);
+  // Answered as an ordinary 2-register read, not acknowledged.
+  EXPECT_THAT(transfer(door, FIRST_HALF | 0x05, SUB_SERIAL, SERIAL, 14), ::testing::ElementsAre(0x8504, 0x0400));
 }
 
 // Like Hoermann's own bus accessory, the first status poll gets an ordinary answer and the next one carries the
@@ -166,7 +165,7 @@ TEST(HoermannHcpTextSensorTest, SerialNumberInTwoHalvesThenTheFirmwareVersion) {
   request_serial(door);
 
   RegisterValues answer = transfer(door, FIRST_HALF | 0x05, SUB_SERIAL, SERIAL, 14);
-  ASSERT_EQ(answer.size(), 8u);
+  ASSERT_EQ(answer.size(), 2u);
   EXPECT_EQ(answer[0], 0x0500);
   EXPECT_EQ(answer[1], 0x04FD);
   EXPECT_EQ(fixture.serial_shown(), "");
@@ -410,19 +409,16 @@ TEST(HoermannHcpTextSensorTest, FirmwareVersionThatIsNotTextIsLoggedNotShown) {
   EXPECT_EQ(door.identity_request_(), 0);
 }
 
-// A repeat of a transfer already taken, as after a lost acknowledgement, is acknowledged again. Answered as a
-// status poll instead, it would carry the key press waiting in the slot.
-TEST(HoermannHcpTextSensorTest, RepeatedTransferIsAcknowledgedNotAnsweredWithAKeyPress) {
+// A repeated transfer, as after a lost acknowledgement, is acknowledged again, not answered with a command.
+TEST(HoermannHcpTextSensorTest, RepeatedTransferIsAcknowledgedNotAnsweredWithACommand) {
   IdentityFixture fixture;
   auto &door = fixture.door;
   run_identity_exchange(door);
   connect_controller(door);
   door.open_door();
 
-  const RegisterValues answer = transfer(door, 0x08, SUB_FIRMWARE, FIRMWARE, 12);
-  EXPECT_EQ(answer[1], 0x04FD);
-  EXPECT_EQ(answer[2], 0x0000);
-  EXPECT_EQ(status_poll(door)[2], 0x0210);
+  EXPECT_THAT(transfer(door, 0x08, SUB_FIRMWARE, FIRMWARE, 12), ::testing::ElementsAre(0x0800, 0x04FD));
+  EXPECT_EQ(status_poll(door)[2], 0x0110);
 }
 
 // An answer belongs to the frame whose write half took the transfer. A frame whose read went elsewhere leaves
@@ -444,29 +440,38 @@ TEST(HoermannHcpTextSensorTest, RequestRidesOnlyOnAStatusPoll) {
   IdentityFixture fixture;
   auto &door = fixture.door;
   status_poll(door);
-  const RegisterValues other = transfer(door, 0x06, 0x19, "\x00\x0F", 2);
+  const RegisterValues other = transfer(door, 0x06, 0x19, "\x00\x0F", 2, 8);
   EXPECT_EQ(other[1] & 0x00FF, 0x0001);
   EXPECT_EQ(status_poll(door, 0x07)[1], 0x0322);
 }
 
-// The request travels in the registers a key press would, so it waits for the press, the hold and the release.
-TEST(HoermannHcpTextSensorTest, RequestWaitsForTheKeyPress) {
+// The request waits for the answer that carries a door command.
+TEST(HoermannHcpTextSensorTest, RequestWaitsForTheDoorCommand) {
   IdentityFixture fixture;
   auto &door = fixture.door;
-  door.key_press_delay_ms_ = 100;
   connect_controller(door);
   status_poll(door);
   door.open_door();
 
-  EXPECT_EQ(status_poll(door)[2], 0x0210);
-  const RegisterValues held = status_poll(door);
-  EXPECT_EQ(held[1], 0x0301);
-  EXPECT_EQ(held[2], 0x0000);
-  door.key_press_delay_ms_ = 0;
-  std::this_thread::sleep_for(KEY_PRESS_ELAPSED);
-  const RegisterValues release = status_poll(door);
-  EXPECT_EQ(release[1], 0x0301);
-  EXPECT_EQ(release[2], 0x0110);
+  const RegisterValues command = status_poll(door);
+  EXPECT_EQ(command[1], 0x0301);
+  EXPECT_EQ(command[2], 0x0110);
+  EXPECT_EQ(status_poll(door)[1], 0x0322);
+}
+
+// The same for the lamp command.
+TEST(HoermannHcpTextSensorTest, RequestWaitsForTheLampCommand) {
+  IdentityFixture fixture;
+  auto &door = fixture.door;
+  connect_controller(door);
+  status_poll(door);
+  door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000));
+  ASSERT_TRUE(door.set_light(true));
+
+  const RegisterValues light = status_poll(door);
+  EXPECT_EQ(light[1], 0x0301);
+  EXPECT_EQ(light[2], LIGHT_ON);
+  EXPECT_EQ(light[3], LIGHT_ON_2);
   EXPECT_EQ(status_poll(door)[1], 0x0322);
 }