Dry Contact Obstruction sensor improvements

[breakingspell]

Ensures the door state is accurately handled if the obstruction sensor is tripped while closing.

Also partially addresses #344.

Prerequisite: #398

[breakingspell]

esphome-ratgdo/components/ratgdo/ratgdo.cpp

Lines 560 to 566 in 0336213

	if (*this->closing_duration > 0) {
	// query state in case we don't get a status message
	set_timeout("door_query_state", (*this->closing_duration + 2) * 1000, [=]() {
	if (*this->door_state != DoorState::CLOSED && *this->door_state != DoorState::STOPPED) {
	this->received(DoorState::CLOSED); // probably missed a status mesage, assume it's closed
	this->query_status(); // query in case we're wrong and it's stopped
	}

This incorrectly sets Closed state after the (obstruction-rolled) cycle's timeout due to DoorState::OPEN not being one of the conditions to check.

At the same time though, query_status() is absent from dry_contact protocol. send_door_state() seems to serve the same role, I brainstormed just mirroring that response, but based on a physical sensor check.

Particularly when the Closed limit sensor is present, it might be best to only declare the door securely closed if the sensor is on. For now i've added the Open condition as a check.

[PaulWieland]

Yes this only applies to the dry contact protocol. Security Plus 1/2 gets its status from a serial data connection to the opener.

The dry contact firmware should depend on both the open and closed limit switches being present for dry contact doors, we do not support or recommend using ratgdo with one or no switches. The query_status isn't implemented because there is no need to "query" a dry contact door since the state of the switches is always known. That's not to say that there isn't a bug being introduced by the timeout.

[breakingspell]

The query_status isn't implemented because there is no need to "query" a dry contact door since the state of the switches is always known. That's not to say that there isn't a bug being introduced by the timeout.

Timing out to a Closed state without the additional sensor/status check from the dry contact firmware seems the core issue. It actually represents a "worst case" scenario where the door reports closed but it's not 😱

In an effort to keep the main ratgdo.cpp as protocol-agnostic as possible, would it be best to mimic the response of the Sec 1/2 query_status() in dry_protocol.cpp to ensure the same corrected status is returned via limit switch check from the firmware?

Actually, query_status() is very rarely called, I had thought it was utilized more often.
Correcting the timeout conditions to consider the open state, or canceling door_query_state timeout once the obstruction sensor is tripped may be the quick fixes, but sensor checks before returning DoorState::CLOSED are still fairly important to implement (likely in #398).

[breakingspell]

This has worked as intended over the last month, in conjunction with the dry contact limit switch PR. There are a few places to look at though:

• We discussed changing this action mapping to evaluate the protocol instead of the presence of obstruction sensors common to all three protocols.

  • This protocol check could also ensure the drycontact firmware doesn't call query_status() inappropriately

• Should the the low-pulse condition stop the door, per report?

  • Referencing this comment, low-pulse is the normal state of the sensor, but this may be specific to the other protocols.
  • I may need context on measuring my obstruction sensor's line if it supports this.

[PaulWieland]

Should the the low-pulse condition stop the door

No. The code for interpreting obstructions is already set and should not be duplicated elsewhere. Add a subscription to watch for changes to ObstructionState.

[breakingspell]

I'm definitely misunderstanding the nature of the low-pulse state. Maybe unnecessary for this PR, noted on the subscription pointer though 🙂

[breakingspell]

Hm, this DoorState::CLOSED does cause problems for the dry contact firmware, since it could be set without the Closed limit switch being active.

For security, it's probably best to only set CLOSED state if the limit sensor agrees