diff --git a/esphome/components/hoermann_hcp/hoermann_hcp.cpp b/esphome/components/hoermann_hcp/hoermann_hcp.cpp index 9ab7014e0e..22f0c12c8c 100644 --- a/esphome/components/hoermann_hcp/hoermann_hcp.cpp +++ b/esphome/components/hoermann_hcp/hoermann_hcp.cpp @@ -1,6 +1,7 @@ #include "hoermann_hcp.h" #include +#include #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(0x0001 | command)); - this->push_command_registers_(registers); + if (status_poll && static_cast(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 ®isters) { - 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 ®isters, 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(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; } diff --git a/esphome/components/hoermann_hcp/hoermann_hcp.h b/esphome/components/hoermann_hcp/hoermann_hcp.h index 9ebb717d0c..c16a0890ef 100644 --- a/esphome/components/hoermann_hcp/hoermann_hcp.h +++ b/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 ®isters); + // 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}; diff --git a/esphome/components/hoermann_hcp/light/hoermann_hcp_light.cpp b/esphome/components/hoermann_hcp/light/hoermann_hcp_light.cpp index d3d784928d..f7e40b8921 100644 --- a/esphome/components/hoermann_hcp/light/hoermann_hcp_light.cpp +++ b/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); diff --git a/tests/components/hoermann_hcp/button/hoermann_hcp_button_test.cpp b/tests/components/hoermann_hcp/button/hoermann_hcp_button_test.cpp index 9c38bc1708..3a6fd1ef90 100644 --- a/tests/components/hoermann_hcp/button/hoermann_hcp_button_test.cpp +++ b/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 diff --git a/tests/components/hoermann_hcp/common.h b/tests/components/hoermann_hcp/common.h index bc9975f2e8..d57841b2c7 100644 --- a/tests/components/hoermann_hcp/common.h +++ b/tests/components/hoermann_hcp/common.h @@ -1,7 +1,5 @@ #pragma once -#include #include -#include #include #include #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 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 poll_command(HoermannHcp &door) { const RegisterValues response = status_answer(door); EXPECT_EQ(response.size(), 8u); @@ -53,29 +52,24 @@ inline std::pair 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_; }; diff --git a/tests/components/hoermann_hcp/cover/hoermann_hcp_cover_test.cpp b/tests/components/hoermann_hcp/cover/hoermann_hcp_cover_test.cpp index 43ca47edb2..efe0f3c50e 100644 --- a/tests/components/hoermann_hcp/cover/hoermann_hcp_cover_test.cpp +++ b/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 diff --git a/tests/components/hoermann_hcp/hoermann_hcp_test.cpp b/tests/components/hoermann_hcp/hoermann_hcp_test.cpp index 1cc5301b4a..82121d038e 100644 --- a/tests/components/hoermann_hcp/hoermann_hcp_test.cpp +++ b/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})); diff --git a/tests/components/hoermann_hcp/light/hoermann_hcp_light_test.cpp b/tests/components/hoermann_hcp/light/hoermann_hcp_light_test.cpp index 75b44a3af6..29b791aa4d 100644 --- a/tests/components/hoermann_hcp/light/hoermann_hcp_light_test.cpp +++ b/tests/components/hoermann_hcp/light/hoermann_hcp_light_test.cpp @@ -11,6 +11,11 @@ namespace esphome::hoermann_hcp::testing { namespace { +// A status broadcast with the door at the given position and state that also carries the lamp register. +RegisterValues door_broadcast(uint16_t position, uint16_t state, uint16_t lamp = 0x0000) { + return make_registers({0x0000, position, state, 0x0000, 0x0000, 0x0000, lamp}); +} + // Counts how often the platform is asked to write, so a publish that re-triggers itself becomes visible. class CountingHoermannHcpLight : public HoermannHcpLight { public: @@ -92,136 +97,423 @@ TEST(HoermannHcpLightTest, LampStateIsDecodedFromTheBroadcast) { EXPECT_FALSE(door.is_light_on()); } -// The lamp command is the only one that drives the second command register, on both halves of the press. -TEST(HoermannHcpLightTest, LampCommandUsesTheSecondRegister) { +// A request sends one command, then nothing until the lamp reports. +TEST(HoermannHcpLightTest, RequestSendsOneCommandUntilTheLampReports) { TestableHoermannHcp door; connect_controller(door); - ASSERT_FALSE(door.is_light_on()); - ASSERT_TRUE(door.toggle_light()); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + ASSERT_TRUE(door.set_light(true)); - auto [pressed, pressed_2] = poll_command(door); - EXPECT_EQ(pressed, 0x0100); - EXPECT_EQ(pressed_2, 0x0200); + auto [light, light_2] = poll_command(door); + EXPECT_EQ(light, LIGHT_ON); + EXPECT_EQ(light_2, LIGHT_ON_2); - std::this_thread::sleep_for(KEY_PRESS_ELAPSED); - auto [released, released_2] = poll_command(door); - EXPECT_EQ(released, 0x0800); - EXPECT_EQ(released_2, 0x0200); + // Asking again sends no second command. + ASSERT_TRUE(door.set_light(true)); + auto [waiting, waiting_2] = poll_command(door); + EXPECT_EQ(waiting, 0x0000); + EXPECT_EQ(waiting_2, 0x0000); - // The command is spent, so the next poll carries nothing. + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0010)); + EXPECT_FALSE(door.light_requested_); + EXPECT_TRUE(door.is_light_heading_on()); auto [idle, idle_2] = poll_command(door); EXPECT_EQ(idle, 0x0000); EXPECT_EQ(idle_2, 0x0000); } -// Toggling the lamp must not disturb a cover position the door is still travelling to. -TEST(HoermannHcpLightTest, LampToggleKeepsTheCoverTarget) { +// The wait for the lamp report starts at the fetch and ends with the report. +TEST(HoermannHcpLightTest, FetchStartsTheWaitForTheLamp) { TestableHoermannHcp door; connect_controller(door); - // Position 60/200 = 0.3 while opening, so a 0.5 target is armed and under way. - door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0100})); - ASSERT_TRUE(door.set_position(0.5f)); - consume_command(door); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + ASSERT_TRUE(door.set_light(true)); + EXPECT_FALSE(door.light_command_sent_); + const uint32_t requested_at = door.light_since_; + std::this_thread::sleep_for(std::chrono::milliseconds(2)); - ASSERT_TRUE(door.toggle_light()); - consume_command(door); - - // Past the target: the door still has to be stopped despite the lamp command in between. - door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0078, 0x0100})); - auto [pressed, pressed_2] = poll_command(door); - EXPECT_EQ(pressed, 0x0240); // COMMAND_IMPULSE - EXPECT_EQ(pressed_2, 0x0000); -} - -// A lamp toggle occupies the single command slot, so a target stop falling due while it waits to be fetched -// has to wait too. The target stays armed and the stop goes out on the next position report, which costs the -// door a little overshoot but never loses the stop. -TEST(HoermannHcpLightTest, LampToggleDelaysButDoesNotLoseTheTargetStop) { - TestableHoermannHcp door; - connect_controller(door); - // Position 60/200 = 0.3 while opening, so a 0.5 target is armed and under way. - door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0100})); - ASSERT_TRUE(door.set_position(0.5f)); - consume_command(door); - - ASSERT_TRUE(door.toggle_light()); - // The door passes the target while the lamp toggle still holds the slot, so the lamp goes out first. - door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0078, 0x0100})); - auto [pressed, pressed_2] = poll_command(door); - EXPECT_EQ(pressed, 0x0100); - EXPECT_EQ(pressed_2, 0x0200); - std::this_thread::sleep_for(KEY_PRESS_ELAPSED); poll_command(door); + EXPECT_TRUE(door.light_command_sent_); + EXPECT_GT(door.light_since_, requested_at); - // The target survived the refusal, so the next position report still stops the door. - door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0079, 0x0100})); - auto [stop, stop_2] = poll_command(door); - EXPECT_EQ(stop, 0x0240); // COMMAND_IMPULSE - EXPECT_EQ(stop_2, 0x0000); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0010)); + EXPECT_FALSE(door.light_requested_); + EXPECT_FALSE(door.light_command_sent_); } -// The target's start deadline is its own, so toggling the lamp cannot keep a stale target alive. -TEST(HoermannHcpLightTest, LampToggleDoesNotExtendTheTargetWatchdog) { +// A request the lamp already meets, or one taken back before the fetch, sends nothing. +TEST(HoermannHcpLightTest, RequestTheLampAlreadyMeetsSendsNothing) { + TestableHoermannHcp door; + connect_controller(door); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + + ASSERT_TRUE(door.set_light(false)); + EXPECT_FALSE(door.light_requested_); + + ASSERT_TRUE(door.set_light(true)); + ASSERT_TRUE(door.set_light(false)); + EXPECT_FALSE(door.light_requested_); + EXPECT_FALSE(door.is_light_heading_on()); + auto [idle, idle_2] = poll_command(door); + EXPECT_EQ(idle, 0x0000); + EXPECT_EQ(idle_2, 0x0000); +} + +// A request reversed after the fetch sends again once the first lands. +TEST(HoermannHcpLightTest, RequestReversedAfterTheFetchSendsAgainOnceTheFirstLands) { + TestableHoermannHcp door; + connect_controller(door); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + ASSERT_TRUE(door.set_light(true)); + EXPECT_EQ(poll_command(door).first, LIGHT_ON); + + ASSERT_TRUE(door.set_light(false)); + EXPECT_FALSE(door.is_light_heading_on()); + // Not decided again before the first command is reported. + EXPECT_EQ(poll_command(door).first, 0x0000); + + // It lands, away from the request. + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0010)); + EXPECT_TRUE(door.light_requested_); + EXPECT_EQ(poll_command(door).first, LIGHT_OFF); + + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + EXPECT_FALSE(door.light_requested_); + EXPECT_EQ(poll_command(door).first, 0x0000); +} + +// A lamp gone unknown drops the request, so no command is decided against a stale state. +TEST(HoermannHcpLightTest, RequestIsNotSentAgainstAStaleLamp) { + TestableHoermannHcp door; + connect_controller(door); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + ASSERT_TRUE(door.set_light(true)); + + door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000})); + ASSERT_FALSE(door.is_light_known()); + + EXPECT_FALSE(door.light_requested_); + auto [idle, idle_2] = poll_command(door); + EXPECT_EQ(idle, 0x0000); + EXPECT_EQ(idle_2, 0x0000); +} + +// The lamp command, like a door command, only goes out in a status answer. +TEST(HoermannHcpLightTest, LampCommandWaitsForAStatusPoll) { + TestableHoermannHcp door; + connect_controller(door); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + ASSERT_TRUE(door.set_light(true)); + + // 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_FALSE(door.light_command_sent_); + + EXPECT_EQ(poll_command(door).first, LIGHT_ON); +} + +// A lamp switched at the door without a request is followed, never switched back. +TEST(HoermannHcpLightTest, DoorSideLampChangeWithoutARequestSendsNothing) { + TestableHoermannHcp door; + connect_controller(door); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0010)); + EXPECT_TRUE(door.is_light_heading_on()); + EXPECT_EQ(poll_command(door).first, 0x0000); + + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + EXPECT_FALSE(door.is_light_heading_on()); + EXPECT_EQ(poll_command(door).first, 0x0000); +} + +// Decided at the fetch: a lamp already switched to the request at the door gets nothing. +TEST(HoermannHcpLightTest, DoorSideLampChangeToTheRequestBeforeTheFetchSendsNothing) { + TestableHoermannHcp door; + connect_controller(door); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + ASSERT_TRUE(door.set_light(true)); + + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0010)); + + EXPECT_FALSE(door.light_requested_); + EXPECT_EQ(poll_command(door).first, 0x0000); +} + +// An unreported command is given up after the timeout and not sent again. +TEST(HoermannHcpLightTest, WatchdogGivesUpOnAnUnreportedCommand) { TestableHoermannHcp door; door.connection_timeout_ms_ = 20; connect_controller(door); - // The door is closing, so an opening target is armed but not yet under way. - door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0200})); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + ASSERT_TRUE(door.set_light(true)); + EXPECT_EQ(poll_command(door).first, LIGHT_ON); + + std::this_thread::sleep_for(std::chrono::milliseconds(30)); + // The broadcast keeps the connection alive. + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + door.update(); + ASSERT_TRUE(door.is_valid()); + + EXPECT_FALSE(door.light_requested_); + EXPECT_FALSE(door.light_command_sent_); + EXPECT_FALSE(door.is_light_heading_on()); + EXPECT_EQ(poll_command(door).first, 0x0000); +} + +// A door command goes before a pending lamp command. +TEST(HoermannHcpLightTest, DoorCommandGoesBeforeTheLampCommand) { + TestableHoermannHcp door; + connect_controller(door); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + ASSERT_TRUE(door.set_light(true)); + ASSERT_TRUE(door.close_door()); + + EXPECT_EQ(poll_command(door).first, 0x0120); // COMMAND_CLOSE + // The lamp waits for the door to rest: the motor ignores it while moving and may switch it as it starts. + EXPECT_EQ(poll_command(door).first, 0x0000); + door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0064, 0x0200, 0x0000, 0x0000, 0x0000, 0x0000})); + EXPECT_EQ(poll_command(door).first, 0x0000); + door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000, 0x0000, 0x0000, 0x0000, 0x0000})); + auto [light, light_2] = poll_command(door); + EXPECT_EQ(light, LIGHT_ON); + EXPECT_EQ(light_2, LIGHT_ON_2); + EXPECT_EQ(poll_command(door).first, 0x0000); +} + +// Switching a lit lamp off sends the off command. +TEST(HoermannHcpLightTest, OffSendsTheOffCommand) { + TestableHoermannHcp door; + connect_controller(door); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0010)); + ASSERT_TRUE(door.set_light(false)); + auto [light, light_2] = poll_command(door); + EXPECT_EQ(light, LIGHT_OFF); + EXPECT_EQ(light_2, LIGHT_OFF_2); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + EXPECT_FALSE(door.is_light_heading_on()); + EXPECT_EQ(poll_command(door).first, 0x0000); +} + +// A command sent again after the lamp moved away from a changed request gets its own deadline. +TEST(HoermannHcpLightTest, SendingAgainRestartsTheDeadline) { + TestableHoermannHcp door; + // Only the second wait has to stay under the timeout; a late wakeup only lengthens the first. + door.connection_timeout_ms_ = 500; + connect_controller(door); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + ASSERT_TRUE(door.set_light(true)); + EXPECT_EQ(poll_command(door).first, LIGHT_ON); + std::this_thread::sleep_for(std::chrono::milliseconds(450)); + ASSERT_TRUE(door.set_light(false)); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0010)); // the first command lands + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + connect_controller(door); + door.update(); + EXPECT_EQ(poll_command(door).first, LIGHT_OFF); +} + +// A lamp command held while the door starts and moves is not given up while it waits. +TEST(HoermannHcpLightTest, LampCommandHeldForTheDoorIsNotGivenUp) { + TestableHoermannHcp door; + door.connection_timeout_ms_ = 20; + connect_controller(door); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + ASSERT_TRUE(door.set_light(true)); + ASSERT_TRUE(door.close_door()); + EXPECT_EQ(poll_command(door).first, 0x0120); // COMMAND_CLOSE + std::this_thread::sleep_for(std::chrono::milliseconds(30)); + connect_controller(door); + door.update(); + EXPECT_TRUE(door.light_requested_); + door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0064, 0x0200, 0x0000, 0x0000, 0x0000, 0x0000})); + EXPECT_EQ(poll_command(door).first, 0x0000); + std::this_thread::sleep_for(std::chrono::milliseconds(30)); + connect_controller(door); + door.update(); + EXPECT_TRUE(door.light_requested_); + door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000, 0x0000, 0x0000, 0x0000, 0x0000})); + EXPECT_EQ(poll_command(door).first, LIGHT_ON); +} + +// The motor ignores the lamp while its door moves, so a request then goes out once the door rests. +TEST(HoermannHcpLightTest, LampWaitsForAMovingDoorToRest) { + TestableHoermannHcp door; + connect_controller(door); + door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0064, 0x0200, 0x0000, 0x0000, 0x0000, 0x0010})); + ASSERT_TRUE(door.set_light(false)); + EXPECT_EQ(poll_command(door).first, 0x0000); + door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000, 0x0000, 0x0000, 0x0000, 0x0010})); + EXPECT_EQ(poll_command(door).first, LIGHT_OFF); +} + +// A lamp the motor switches on as the door starts already meets the request, so nothing is sent. +TEST(HoermannHcpLightTest, LampSwitchedOnByTheStartGetsNoCommand) { + TestableHoermannHcp door; + connect_controller(door); + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + ASSERT_TRUE(door.set_light(true)); + ASSERT_TRUE(door.close_door()); + + EXPECT_EQ(poll_command(door).first, 0x0120); // COMMAND_CLOSE + EXPECT_EQ(poll_command(door).first, 0x0000); + door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0064, 0x0200, 0x0000, 0x0000, 0x0000, 0x0010})); + EXPECT_TRUE(door.is_light_on()); + EXPECT_EQ(poll_command(door).first, 0x0000); +} + +// A target stop goes out before a pending lamp command. +TEST(HoermannHcpLightTest, LampRequestDoesNotDelayTheTargetStop) { + TestableHoermannHcp door; + connect_controller(door); + // Stopped at 60/200 = 0.3, so a 0.5 target is armed, then the door opens. + door.on_write_registers(BROADCAST_REG, door_broadcast(0x003C, 0x0000)); + ASSERT_TRUE(door.set_position(0.5f)); + consume_command(door); + door.on_write_registers(BROADCAST_REG, door_broadcast(0x003E, 0x0100)); + + ASSERT_TRUE(door.set_light(true)); + door.on_write_registers(BROADCAST_REG, door_broadcast(0x0078, 0x0100)); + auto [stop, stop_2] = poll_command(door); + EXPECT_EQ(stop, 0x0140); // COMMAND_IMPULSE + EXPECT_EQ(stop_2, 0x0000); + // The lamp follows once the door rests. + EXPECT_EQ(poll_command(door).first, 0x0000); + door.on_write_registers(BROADCAST_REG, door_broadcast(0x0082, 0x0000)); + auto [light, light_2] = poll_command(door); + EXPECT_EQ(light, LIGHT_ON); + EXPECT_EQ(light_2, LIGHT_ON_2); +} + +// The target's start deadline is its own, so switching the lamp cannot keep a stale target alive. +TEST(HoermannHcpLightTest, LampCommandDoesNotExtendTheTargetWatchdog) { + TestableHoermannHcp door; + door.connection_timeout_ms_ = 20; + door.start_window_ms_ = 20; + connect_controller(door); + // The door is stopped, so an opening target is armed but not yet under way. + door.on_write_registers(BROADCAST_REG, door_broadcast(0x003C, 0x0000)); ASSERT_TRUE(door.set_position(0.5f)); consume_command(door); std::this_thread::sleep_for(std::chrono::milliseconds(30)); - ASSERT_TRUE(door.toggle_light()); + // The door never started, so the start window closes along with the target. + consume_command(door); + door.update(); + ASSERT_TRUE(door.set_light(true)); consume_command(door); door.update(); // The target expired on its own schedule, so a later opening move runs freely. - door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0050, 0x0100})); - door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0078, 0x0100})); - auto [pressed, pressed_2] = poll_command(door); - EXPECT_EQ(pressed, 0x0000); - EXPECT_EQ(pressed_2, 0x0000); + door.on_write_registers(BROADCAST_REG, door_broadcast(0x0050, 0x0100)); + door.on_write_registers(BROADCAST_REG, door_broadcast(0x0078, 0x0100)); + auto [idle, idle_2] = poll_command(door); + EXPECT_EQ(idle, 0x0000); + EXPECT_EQ(idle_2, 0x0000); } -// Without a bus controller the command cannot be delivered, and the caller is told. -TEST(HoermannHcpLightTest, LampCommandIsRefusedWhileDisconnected) { +// A lamp held while the door moves is not given up, and leaves the cover target alone. +TEST(HoermannHcpLightTest, HeldLampKeepsTheCoverTarget) { + TestableHoermannHcp door; + door.connection_timeout_ms_ = 20; + connect_controller(door); + // Stopped at 60/200 = 0.3, so a 0.5 target is armed, then the door opens. + door.on_write_registers(BROADCAST_REG, door_broadcast(0x003C, 0x0000)); + ASSERT_TRUE(door.set_position(0.5f)); + consume_command(door); + door.on_write_registers(BROADCAST_REG, door_broadcast(0x003E, 0x0100)); + + ASSERT_TRUE(door.set_light(true)); + EXPECT_EQ(poll_command(door).first, 0x0000); + std::this_thread::sleep_for(std::chrono::milliseconds(30)); + door.on_write_registers(BROADCAST_REG, door_broadcast(0x0050, 0x0100)); + door.update(); + ASSERT_TRUE(door.light_requested_); + + // The target survived, so the door is still stopped on the way. + door.on_write_registers(BROADCAST_REG, door_broadcast(0x0078, 0x0100)); + auto [stop, stop_2] = poll_command(door); + EXPECT_EQ(stop, 0x0140); // COMMAND_IMPULSE + EXPECT_EQ(stop_2, 0x0000); +} + +// A lost connection clears both the target and the lamp request. +TEST(HoermannHcpLightTest, ConnectionLossClearsTheTargetAndTheLampRequest) { + TestableHoermannHcp door; + door.connection_timeout_ms_ = 20; + connect_controller(door); + // Stopped at 60/200 = 0.3, so a 0.5 target is armed, then the door opens. + door.on_write_registers(BROADCAST_REG, door_broadcast(0x003C, 0x0000)); + ASSERT_TRUE(door.set_position(0.5f)); + consume_command(door); + door.on_write_registers(BROADCAST_REG, door_broadcast(0x003E, 0x0100)); + ASSERT_TRUE(door.set_light(true)); + + std::this_thread::sleep_for(std::chrono::milliseconds(30)); + door.update(); + ASSERT_FALSE(door.is_valid()); + EXPECT_FALSE(door.light_requested_); + + // Back on the bus and past the old target, lamp still off: nothing is sent. + connect_controller(door); + door.on_write_registers(BROADCAST_REG, door_broadcast(0x0078, 0x0100)); + auto [idle, idle_2] = poll_command(door); + EXPECT_EQ(idle, 0x0000); + EXPECT_EQ(idle_2, 0x0000); +} + +// Without a bus controller the request cannot be delivered, and the caller is told. +TEST(HoermannHcpLightTest, LampRequestIsRefusedWhileDisconnected) { HoermannHcp door; - EXPECT_FALSE(door.toggle_light()); + EXPECT_FALSE(door.set_light(true)); } -// Switching the entity on sends one toggle, and the door's own report does not send a second. -TEST(HoermannHcpLightPlatformTest, CommandTogglesOnceAndSettles) { +// A lamp that has not been reported cannot be requested. +TEST(HoermannHcpLightTest, LampRequestIsRefusedUntilTheLampIsReported) { + TestableHoermannHcp door; + connect_controller(door); + EXPECT_FALSE(door.set_light(true)); + + door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); + EXPECT_TRUE(door.set_light(true)); +} + +// Switching the entity on sends one command, and the door's own report does not send a second. +TEST(HoermannHcpLightPlatformTest, CommandIsSentOnceAndSettles) { LightFixture fixture; fixture.bring_up(); fixture.command(true); - auto [pressed, pressed_2] = poll_command(fixture.door); - EXPECT_EQ(pressed, 0x0100); - EXPECT_EQ(pressed_2, 0x0200); - std::this_thread::sleep_for(KEY_PRESS_ELAPSED); - poll_command(fixture.door); // release, clearing the slot + auto [light, light_2] = poll_command(fixture.door); + EXPECT_EQ(light, LIGHT_ON); + EXPECT_EQ(light_2, LIGHT_ON_2); - // The lamp is now on, and the resulting broadcast must not queue another toggle. + // The lamp is now on, and the resulting broadcast must not send another command. fixture.report_lamp(true); EXPECT_TRUE(fixture.entity_on()); + EXPECT_FALSE(fixture.door.light_requested_); auto [idle, idle_2] = poll_command(fixture.door); EXPECT_EQ(idle, 0x0000); EXPECT_EQ(idle_2, 0x0000); } -// A broadcast arriving while a toggle is queued must not reconcile against the not-yet-inverted lamp, which -// would cancel the user's own command. -TEST(HoermannHcpLightPlatformTest, BroadcastDuringPendingToggleKeepsTheCommand) { +// A broadcast before the fetch must not take the request back. +TEST(HoermannHcpLightPlatformTest, BroadcastDuringPendingRequestKeepsTheCommand) { LightFixture fixture; fixture.bring_up(); fixture.command(true); - ASSERT_TRUE(fixture.door.is_light_toggle_pending_()); + ASSERT_TRUE(fixture.door.light_requested_); - // A door movement sets changed_, firing the state callback while the toggle is still queued. - fixture.report_broadcast(make_registers({0x0000, 0x0064, 0x0100})); + // A door movement fires the state callback before the fetch. + fixture.report_broadcast(door_broadcast(0x0064, 0x0100)); - EXPECT_TRUE(fixture.door.is_light_toggle_pending_()); + EXPECT_TRUE(fixture.door.light_requested_); EXPECT_TRUE(fixture.entity_on()); } @@ -246,87 +538,83 @@ TEST(HoermannHcpLightPlatformTest, RefusedCommandRepublishesTheLamp) { EXPECT_FALSE(fixture.entity_on()); } -// A reversing press once the toggle is already on the wire cannot stop it, so the entity has to end up -// showing the lamp rather than the request that was refused. -TEST(HoermannHcpLightPlatformTest, RefusedPressAfterFetchShowsWhereTheLampIsHeading) { +// A lamp gone unknown drops the request; the entity follows the next report. +TEST(HoermannHcpLightPlatformTest, LampBecomingUnknownDropsTheRequest) { LightFixture fixture; fixture.bring_up(); fixture.command(true); - poll_command(fixture.door); // the controller fetches the press, so it can no longer be cancelled - ASSERT_TRUE(fixture.door.is_light_toggle_pending_()); + EXPECT_EQ(poll_command(fixture.door).first, LIGHT_ON); - fixture.command(false); - EXPECT_TRUE(fixture.entity_on()); - - // A door movement while the refused toggle is still on the wire must not pull the entity back either. fixture.report_broadcast(make_registers({0x0000, 0x0064, 0x0100})); - EXPECT_TRUE(fixture.entity_on()); + EXPECT_FALSE(fixture.door.light_requested_); + EXPECT_FALSE(fixture.door.light_command_sent_); + EXPECT_TRUE(fixture.output.status_has_warning()); - // The toggle lands and the door confirms it; the entity must already agree. - fixture.report_lamp(true); - EXPECT_TRUE(fixture.entity_on()); + // The door ignored the command. + fixture.report_lamp(false); + EXPECT_FALSE(fixture.entity_on()); + EXPECT_EQ(poll_command(fixture.door).first, 0x0000); } -// The lamp is only reported some time after the key press is released, so an unrelated door broadcast in -// that gap must not publish the state the lamp is about to leave. +// A command never fetched is dropped and not sent once polling resumes. +TEST(HoermannHcpLightPlatformTest, UnfetchedRequestIsDroppedAndNeverSent) { + LightFixture fixture; + fixture.door.connection_timeout_ms_ = 20; + fixture.bring_up(); + + fixture.command(true); + ASSERT_TRUE(fixture.door.light_requested_); + EXPECT_TRUE(fixture.entity_on()); + + // Broadcasts keep the connection up, but nothing polls. + std::this_thread::sleep_for(std::chrono::milliseconds(30)); + fixture.report_lamp(false); + ASSERT_TRUE(fixture.door.is_valid()); + + EXPECT_FALSE(fixture.door.light_requested_); + EXPECT_FALSE(fixture.entity_on()); + auto [idle, idle_2] = poll_command(fixture.door); + EXPECT_EQ(idle, 0x0000); + EXPECT_EQ(idle_2, 0x0000); +} + +// A door broadcast before the lamp reports must not publish the state the lamp is about to leave. TEST(HoermannHcpLightPlatformTest, DoorMovementDoesNotFlipTheEntityBeforeTheLampReports) { LightFixture fixture; fixture.bring_up(); fixture.command(true); poll_command(fixture.door); - std::this_thread::sleep_for(KEY_PRESS_ELAPSED); - poll_command(fixture.door); // release, so nothing is pending any more - ASSERT_FALSE(fixture.door.is_light_toggle_pending_()); + ASSERT_TRUE(fixture.door.light_command_sent_); ASSERT_FALSE(fixture.door.is_light_on()); // the lamp has still not been reported - fixture.report_broadcast(make_registers({0x0000, 0x0064, 0x0100})); + fixture.report_broadcast(door_broadcast(0x0064, 0x0100)); EXPECT_TRUE(fixture.entity_on()); } -// A toggle the controller never fetches is eventually dropped, and nothing else will ever report the lamp -// moving, so the entity has to be brought back to what the lamp actually is. -TEST(HoermannHcpLightPlatformTest, DroppedToggleReturnsTheEntityToTheLamp) { +// A request lost with the connection leaves the entity on the lamp. +TEST(HoermannHcpLightPlatformTest, RequestLostWithTheConnectionReturnsTheEntityToTheLamp) { LightFixture fixture; fixture.door.connection_timeout_ms_ = 20; fixture.bring_up(); fixture.command(true); - ASSERT_TRUE(fixture.door.is_light_toggle_pending_()); - EXPECT_TRUE(fixture.entity_on()); - - // The controller keeps broadcasting but never fetches the command, so the connection stays up. - std::this_thread::sleep_for(std::chrono::milliseconds(30)); - fixture.door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); - fixture.pump(); - - EXPECT_FALSE(fixture.door.is_light_toggle_pending_()); - EXPECT_FALSE(fixture.entity_on()); -} - -// Losing the bus controller discards the queued toggle too, so the entity must not keep showing it once the -// controller is back and still reporting the lamp unchanged. -TEST(HoermannHcpLightPlatformTest, ToggleLostWithTheConnectionReturnsTheEntityToTheLamp) { - LightFixture fixture; - fixture.door.connection_timeout_ms_ = 20; - fixture.bring_up(); - - fixture.command(true); - ASSERT_TRUE(fixture.door.is_light_toggle_pending_()); + ASSERT_TRUE(fixture.door.light_requested_); std::this_thread::sleep_for(std::chrono::milliseconds(30)); - fixture.pump(); // the connection times out and the command goes with it + fixture.pump(); // the connection times out and the request goes with it ASSERT_FALSE(fixture.door.is_valid()); connect_controller(fixture.door); fixture.report_lamp(false); EXPECT_FALSE(fixture.entity_on()); + EXPECT_EQ(poll_command(fixture.door).first, 0x0000); } // The lamp can be switched at the door while the bus is quiet, so what was read before an outage must not -// decide whether a toggle is needed after it. +// decide whether a command is needed after it. TEST(HoermannHcpLightPlatformTest, LampIsNotTrustedAcrossAConnectionLoss) { LightFixture fixture; fixture.door.connection_timeout_ms_ = 20; @@ -362,25 +650,23 @@ TEST(HoermannHcpLightPlatformTest, UnreportedLampIsFlaggedOnTheEntity) { EXPECT_FALSE(fixture.output.status_has_warning()); } -// Two outstanding toggles leave the lamp where it started, so a third tap has to be judged against that and -// withdraw the one still waiting rather than deciding nothing is needed. -TEST(HoermannHcpLightPlatformTest, ThirdTapWithTwoTogglesOutstandingIsHonoured) { +// On, off, on while the command is out ends on the last request with one command. +TEST(HoermannHcpLightPlatformTest, QuickTapsSettleOnTheLastRequest) { LightFixture fixture; fixture.bring_up(); fixture.command(true); - poll_command(fixture.door); - std::this_thread::sleep_for(KEY_PRESS_ELAPSED); - poll_command(fixture.door); // the first toggle is released but not reported back + EXPECT_EQ(poll_command(fixture.door).first, LIGHT_ON); fixture.command(false); - ASSERT_TRUE(fixture.door.is_light_toggle_pending_()); - ASSERT_EQ(fixture.door.light_toggles_in_flight_, 2); - - // Two toggles cancel out, so asking for on again means withdrawing the second one. + EXPECT_FALSE(fixture.entity_on()); fixture.command(true); - EXPECT_FALSE(fixture.door.is_light_toggle_pending_()); - EXPECT_EQ(fixture.door.light_toggles_in_flight_, 1); EXPECT_TRUE(fixture.entity_on()); + EXPECT_EQ(poll_command(fixture.door).first, 0x0000); + + fixture.report_lamp(true); + EXPECT_FALSE(fixture.door.light_requested_); + EXPECT_TRUE(fixture.entity_on()); + EXPECT_EQ(poll_command(fixture.door).first, 0x0000); } // The boot replay is the first write and nothing else, so a real command arriving before the hub's next poll @@ -395,9 +681,9 @@ TEST(HoermannHcpLightPlatformTest, CommandBeforeTheFirstPollIsNotMistakenForTheB ASSERT_TRUE(fixture.door.is_light_known()); fixture.command(true); - auto [pressed, pressed_2] = poll_command(fixture.door); - EXPECT_EQ(pressed, 0x0100); // COMMAND_TOGGLE_LAMP - EXPECT_EQ(pressed_2, 0x0200); + auto [light, light_2] = poll_command(fixture.door); + EXPECT_EQ(light, LIGHT_ON); + EXPECT_EQ(light_2, LIGHT_ON_2); } // On boot the restored state is replayed through write_state() before the lamp has ever been read. A lamp @@ -410,6 +696,7 @@ TEST(HoermannHcpLightPlatformTest, RestoredStateOnBootDoesNotCommandTheLamp) { ASSERT_TRUE(fixture.door.is_light_on()); fixture.settle(); + EXPECT_FALSE(fixture.door.light_requested_); auto [idle, idle_2] = poll_command(fixture.door); EXPECT_EQ(idle, 0x0000); EXPECT_EQ(idle_2, 0x0000); @@ -435,39 +722,34 @@ TEST(HoermannHcpLightPlatformTest, RequestBeforeTheLampIsReportedDoesNotCommandT EXPECT_FALSE(fixture.entity_on()); } -// A toggle that has been released onto the wire is no longer pending, but the lamp has not reported it yet. -// A reversing request in that window is a real request and has to be sent, not swallowed. -TEST(HoermannHcpLightPlatformTest, ReversingRequestAfterReleaseQueuesASecondToggle) { +// A reversal after the fetch shows at once and switches back once the first lands. +TEST(HoermannHcpLightPlatformTest, ReversingRequestAfterFetchSwitchesBackOnceTheFirstLands) { LightFixture fixture; fixture.bring_up(); fixture.command(true); - poll_command(fixture.door); - std::this_thread::sleep_for(KEY_PRESS_ELAPSED); - poll_command(fixture.door); // released, so nothing is pending and the lamp is still unreported - ASSERT_FALSE(fixture.door.is_light_toggle_pending_()); + EXPECT_EQ(poll_command(fixture.door).first, LIGHT_ON); ASSERT_FALSE(fixture.door.is_light_on()); fixture.command(false); - auto [pressed, pressed_2] = poll_command(fixture.door); - EXPECT_EQ(pressed, 0x0100); // COMMAND_TOGGLE_LAMP - EXPECT_EQ(pressed_2, 0x0200); EXPECT_FALSE(fixture.entity_on()); + EXPECT_EQ(poll_command(fixture.door).first, 0x0000); - // The first toggle lands and is reported, but the entity is already heading for off. + // The first command lands and is reported, but the entity is already heading for off. fixture.report_lamp(true); EXPECT_FALSE(fixture.entity_on()); + auto [light, light_2] = poll_command(fixture.door); + EXPECT_EQ(light, LIGHT_OFF); + EXPECT_EQ(light_2, LIGHT_OFF_2); - // The second toggle lands too, and the lamp finally agrees with the request. - std::this_thread::sleep_for(KEY_PRESS_ELAPSED); - poll_command(fixture.door); + // The second command lands too, and the lamp finally agrees with the request. fixture.report_lamp(false); EXPECT_FALSE(fixture.entity_on()); + EXPECT_FALSE(fixture.door.light_requested_); } -// A refusal that has no toggle on the wire leaves nothing outstanding, so it must not latch the entity -// against the next lamp change the door reports. -TEST(HoermannHcpLightPlatformTest, RefusalWithoutAToggleStillFollowsTheLamp) { +// A refusal leaves no request, so the entity keeps following the lamp. +TEST(HoermannHcpLightPlatformTest, RefusalWithoutARequestStillFollowsTheLamp) { LightFixture fixture; fixture.door.connection_timeout_ms_ = 20; fixture.bring_up(); @@ -476,7 +758,7 @@ TEST(HoermannHcpLightPlatformTest, RefusalWithoutAToggleStillFollowsTheLamp) { fixture.pump(); ASSERT_FALSE(fixture.door.is_valid()); - // Refused because the bus is down, so no toggle is heading for the lamp. + // Refused because the bus is down, so no command is heading for the lamp. fixture.command(true); EXPECT_FALSE(fixture.entity_on()); @@ -486,39 +768,15 @@ TEST(HoermannHcpLightPlatformTest, RefusalWithoutAToggleStillFollowsTheLamp) { EXPECT_TRUE(fixture.entity_on()); } -// A lamp toggle carries no target, so dropping it unfetched must leave the cover's target alone. -TEST(HoermannHcpLightTest, DroppedLampToggleKeepsTheCoverTarget) { - TestableHoermannHcp door; - door.connection_timeout_ms_ = 20; - connect_controller(door); - // Position 60/200 = 0.3 while opening, so a 0.5 target is armed and under way. - door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0100})); - ASSERT_TRUE(door.set_position(0.5f)); - consume_command(door); - - // The controller keeps broadcasting but stops fetching, so the lamp toggle expires on its own. - ASSERT_TRUE(door.toggle_light()); - std::this_thread::sleep_for(std::chrono::milliseconds(30)); - door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0050, 0x0100})); - door.update(); - - // The target survived the lamp toggle being dropped, so the door is still stopped on the way. - door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0078, 0x0100})); - auto [pressed, pressed_2] = poll_command(door); - EXPECT_EQ(pressed, 0x0240); // COMMAND_IMPULSE - EXPECT_EQ(pressed_2, 0x0000); -} - -// A door that takes the key press but never actually switches the lamp must not leave the entity showing the -// request for ever; the wait has to end so the entity can settle back on what the door reports. -TEST(HoermannHcpLightPlatformTest, ToggleTheDoorIgnoresStopsBeingWaitedFor) { +// A command the door never carries out must not leave the entity on the request. +TEST(HoermannHcpLightPlatformTest, CommandTheDoorIgnoresStopsBeingWaitedFor) { LightFixture fixture; fixture.door.connection_timeout_ms_ = 20; fixture.bring_up(); fixture.command(true); - consume_command(fixture.door); // the door takes press and release, then does nothing - ASSERT_FALSE(fixture.door.is_light_toggle_pending_()); + consume_command(fixture.door); // the door takes the command, then does nothing + ASSERT_TRUE(fixture.door.light_command_sent_); EXPECT_TRUE(fixture.entity_on()); std::this_thread::sleep_for(std::chrono::milliseconds(30)); @@ -536,60 +794,50 @@ TEST(HoermannHcpLightPlatformTest, FirstLampReportReachesTheEntity) { ASSERT_FALSE(fixture.door.is_light_known()); // Closed, at rest, lamp off: every field matches the defaults the hub started with. - fixture.report_broadcast(make_registers({0x0000, 0x0000, 0x4000, 0x0000, 0x0000, 0x0000, 0x0000})); + fixture.report_broadcast(door_broadcast(0x0000, 0x4000)); ASSERT_TRUE(fixture.door.is_light_known()); fixture.command(true); - auto [pressed, pressed_2] = poll_command(fixture.door); - EXPECT_EQ(pressed, 0x0100); // COMMAND_TOGGLE_LAMP - EXPECT_EQ(pressed_2, 0x0200); + auto [light, light_2] = poll_command(fixture.door); + EXPECT_EQ(light, LIGHT_ON); + EXPECT_EQ(light_2, LIGHT_ON_2); } -// A lost connection means the door can travel unwatched, so a target left armed would stop it long afterwards. -// Which command happened to be in the slot must not change that. -TEST(HoermannHcpLightTest, ConnectionLossWithALampTogglePendingClearsTheTarget) { - TestableHoermannHcp door; - door.connection_timeout_ms_ = 20; - connect_controller(door); - // Position 60/200 = 0.3 while opening, so a 0.5 target is armed and under way. - door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x003C, 0x0100})); - ASSERT_TRUE(door.set_position(0.5f)); - consume_command(door); - ASSERT_TRUE(door.toggle_light()); - - std::this_thread::sleep_for(std::chrono::milliseconds(30)); - door.update(); - ASSERT_FALSE(door.is_valid()); - - // Back on the bus and travelling past where the target was: nothing should stop the door now. - connect_controller(door); - door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0078, 0x0100})); - auto [pressed, pressed_2] = poll_command(door); - EXPECT_EQ(pressed, 0x0000); - EXPECT_EQ(pressed_2, 0x0000); -} - -// Withdrawing a later toggle must not take the deadline of the one already on the wire with it, or a door -// that never reports the lamp would leave the entity waiting for ever. -TEST(HoermannHcpLightPlatformTest, WithdrawingALaterToggleKeepsTheWatchdogArmed) { +// Changing the request does not move the deadline of the command already out. +TEST(HoermannHcpLightPlatformTest, ChangingTheRequestKeepsTheWatchdogArmed) { LightFixture fixture; fixture.door.connection_timeout_ms_ = 20; fixture.bring_up(); fixture.command(true); - consume_command(fixture.door); // the first toggle is released but never reported back + consume_command(fixture.door); // the command is fetched but never reported back + ASSERT_TRUE(fixture.door.light_command_sent_); + const uint32_t sent_at = fixture.door.light_since_; fixture.command(false); - ASSERT_EQ(fixture.door.light_toggles_in_flight_, 2); - fixture.command(true); // withdraws the second, leaving the first outstanding - ASSERT_EQ(fixture.door.light_toggles_in_flight_, 1); + fixture.command(true); + ASSERT_EQ(fixture.door.light_since_, sent_at); // The door still says nothing about the lamp, so the wait has to time out on its own. std::this_thread::sleep_for(std::chrono::milliseconds(30)); fixture.report_lamp(false); - EXPECT_EQ(fixture.door.light_toggles_in_flight_, 0); + EXPECT_FALSE(fixture.door.light_requested_); EXPECT_FALSE(fixture.entity_on()); } +// Asking again before the fetch does not move the deadline of the pending request. +TEST(HoermannHcpLightPlatformTest, AskingAgainKeepsTheFetchDeadline) { + LightFixture fixture; + fixture.door.connection_timeout_ms_ = 20; + fixture.bring_up(); + + fixture.command(true); + ASSERT_TRUE(fixture.door.light_requested_); + const uint32_t requested_at = fixture.door.light_since_; + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + ASSERT_TRUE(fixture.door.set_light(true)); + EXPECT_EQ(fixture.door.light_since_, requested_at); +} + // A request refused while the lamp is unknown must leave the entity idle. Republishing unconditionally would // re-enter write_state() on every loop, so the platform would never stop asking to be written. TEST(HoermannHcpLightPlatformTest, RefusedRequestLeavesTheEntityIdle) { @@ -605,25 +853,6 @@ TEST(HoermannHcpLightPlatformTest, RefusedRequestLeavesTheEntityIdle) { EXPECT_EQ(fixture.output.writes, settled_writes); } -// A door that acts on the key press and reports the lamp before the release is even fetched leaves nothing -// outstanding. Arming the watchdog on that release anyway would leave it firing on every poll and abandoning -// the next toggle the moment it is queued. -TEST(HoermannHcpLightTest, ReleaseWithNothingOutstandingLeavesTheWatchdogDisarmed) { - TestableHoermannHcp door; - connect_controller(door); - door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); - ASSERT_TRUE(door.toggle_light()); - poll_command(door); // the door is shown the key press - - // The door acts on it and reports the lamp straight away, which settles the count. - door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0010)); - ASSERT_EQ(door.light_toggles_in_flight_, 0); - - std::this_thread::sleep_for(KEY_PRESS_ELAPSED); - poll_command(door); // the release, with nothing left to wait for - EXPECT_EQ(door.light_toggle_released_at_, 0u); -} - // Booting the entity on replays a lit state the door has never confirmed, so it has to be // adopted back to what is known rather than turned into a command. TEST(HoermannHcpLightPlatformTest, RestoredOnStateIsAdoptedNotCommanded) { @@ -637,93 +866,22 @@ TEST(HoermannHcpLightPlatformTest, RestoredOnStateIsAdoptedNotCommanded) { EXPECT_FALSE(fixture.entity_on()); } -// A reversing press before the toggle is fetched cancels it, so the lamp never moves. -TEST(HoermannHcpLightPlatformTest, ReversingPressCancelsTheQueuedToggle) { +// A reversal before the fetch takes the request back, so nothing is sent. +TEST(HoermannHcpLightPlatformTest, ReversingPressBeforeTheFetchSendsNothing) { LightFixture fixture; fixture.bring_up(); fixture.command(true); - ASSERT_TRUE(fixture.door.is_light_toggle_pending_()); + ASSERT_TRUE(fixture.door.light_requested_); fixture.command(false); - EXPECT_FALSE(fixture.door.is_light_toggle_pending_()); + EXPECT_FALSE(fixture.door.light_requested_); EXPECT_FALSE(fixture.entity_on()); // Nothing is left for the controller to fetch, so the lamp stays off as asked. - auto [pressed, pressed_2] = poll_command(fixture.door); - EXPECT_EQ(pressed, 0x0000); - EXPECT_EQ(pressed_2, 0x0000); -} - -// A lamp switched at the door itself is not one of our toggles landing, so a toggle the door has not even -// been shown has to keep counting. -TEST(HoermannHcpLightTest, DoorSideLampChangeLeavesAnUnsentToggleCounted) { - TestableHoermannHcp door; - connect_controller(door); - door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); - ASSERT_TRUE(door.toggle_light()); - - door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0010)); - - EXPECT_EQ(door.light_toggles_in_flight_, 1); - // The toggle still in the slot will invert what the door just reported. - EXPECT_FALSE(door.is_light_heading_on()); -} - -// Once the toggles left over are all still waiting in the slot, nothing the door has seen is outstanding, -// so the wait has to end rather than time out against toggles the door was never shown. -TEST(HoermannHcpLightTest, SettlingTheLastSentToggleEndsTheWait) { - TestableHoermannHcp door; - connect_controller(door); - door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); - ASSERT_TRUE(door.toggle_light()); - consume_command(door); // shown to the door, so the wait for a lamp report starts - ASSERT_TRUE(door.toggle_light()); // queued behind it, never shown - ASSERT_NE(door.light_toggle_released_at_, 0u); - - // The door reports the lamp change the first toggle caused, leaving only the unsent one. - door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0010)); - - ASSERT_EQ(door.light_toggles_in_flight_, 1); - EXPECT_EQ(door.light_toggle_released_at_, 0u); -} - -// The watchdog gives up on the toggles the door was shown, but one still waiting in the command slot is -// going to fire, so it keeps counting. -TEST(HoermannHcpLightTest, WatchdogKeepsAToggleTheDoorHasNotSeen) { - TestableHoermannHcp door; - // Wide enough that the toggle queued after the sleep cannot expire before update() runs. - door.connection_timeout_ms_ = 200; - connect_controller(door); - door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); - ASSERT_TRUE(door.toggle_light()); - consume_command(door); // shown to the door, which then says nothing about the lamp - - std::this_thread::sleep_for(std::chrono::milliseconds(220)); - // Queued just now, so only the wait for the first toggle is overdue. - door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); - ASSERT_TRUE(door.toggle_light()); - door.update(); - - EXPECT_EQ(door.light_toggles_in_flight_, 1); - EXPECT_TRUE(door.is_light_toggle_pending_()); - EXPECT_TRUE(door.is_light_heading_on()); -} - -// Only the parity of the outstanding count says where the lamp is heading, so the count must not run away. -TEST(HoermannHcpLightTest, TogglesAreRefusedOnceTooManyAreOutstanding) { - TestableHoermannHcp door; - connect_controller(door); - door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0000)); - - // The door takes every key press but never reports the lamp, so nothing is ever confirmed. - for (int i = 0; i < 4; i++) { - ASSERT_TRUE(door.toggle_light()); - consume_command(door); - } - - EXPECT_FALSE(door.toggle_light()); - EXPECT_EQ(door.light_toggles_in_flight_, 4); + auto [idle, idle_2] = poll_command(fixture.door); + EXPECT_EQ(idle, 0x0000); + EXPECT_EQ(idle_2, 0x0000); } // A controller that stops carrying the lamp register leaves nothing refreshing it, so the entity has to flag @@ -740,25 +898,25 @@ TEST(HoermannHcpLightPlatformTest, BroadcastWithoutTheLampRegisterMarksItUnknown } // A publish of ours only reaches write_state() a loop pass later. If the lamp changed at the door in that -// gap, the write still carries the old value and must not be taken for a request to invert the lamp. +// gap, the write still carries the old value and must not be taken for a request to switch the lamp back. TEST(HoermannHcpLightPlatformTest, PublishOvertakenByTheLampIsNotARequest) { LightFixture fixture; fixture.bring_up(); - // A door command holds the only command slot, so the request below is refused and the lamp published back. - ASSERT_TRUE(fixture.door.open_door()); + // Lamp unknown, so the request below is refused and published back. + fixture.door.on_write_registers(BROADCAST_REG, make_registers({0x0000, 0x0000, 0x4000})); auto call = fixture.state.make_call(); call.set_state(true); call.perform(); fixture.state.loop(); // the refusal happens here and schedules the publish for a later pass - // The slot frees up and the lamp is switched on at the door before that publish arrives. - consume_command(fixture.door); + // The lamp is reported again, switched on at the door, before that publish arrives. fixture.door.on_write_registers(BROADCAST_REG, lamp_broadcast(0x0010)); fixture.settle(); - EXPECT_EQ(fixture.door.light_toggles_in_flight_, 0); + EXPECT_FALSE(fixture.door.light_requested_); EXPECT_TRUE(fixture.entity_on()); + EXPECT_EQ(poll_command(fixture.door).first, 0x0000); } } // namespace esphome::hoermann_hcp::testing diff --git a/tests/components/hoermann_hcp/text_sensor/hoermann_hcp_text_sensor_test.cpp b/tests/components/hoermann_hcp/text_sensor/hoermann_hcp_text_sensor_test.cpp index 8dff56e2a3..01d5198781 100644 --- a/tests/components/hoermann_hcp/text_sensor/hoermann_hcp_text_sensor_test.cpp +++ b/tests/components/hoermann_hcp/text_sensor/hoermann_hcp_text_sensor_test.cpp @@ -2,7 +2,6 @@ #include #include -#include #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); }