Skip to content

Dry Contact Limit sensor improvements #398

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

breakingspell
Copy link
Contributor

@breakingspell breakingspell commented Apr 2, 2025

All dry-contact setups require two limit switches for the open+closed positions. We can utilize these sensors for additional checks and awareness, especially with the OPEN status potentially representing a partially-opened state.

  • The existing checks for the open+closed state in dry_contact.cpp are updated to check the sensors before ignoring the open/close command.

    • This resolves behavior following a reboot with the door positioned partway, the resulting DoorState::UNKNOWN would cause both cover up+down commands to be declined.
    • It also prevents additional "close" commands while closed from bouncing the door into an inverted state.
  • The limit sensor methods in ratgdo.cpp are updated to manually set the door position for the cover once triggered.

    • This fixes if out-of-sync due to the wall button/obstacle sensor, but requires a "check-in" to a sensor.
      • This could also be fixed or added to by handling the obstruction sensor event to set Opening state.
    • The code states this should ideally be done in drycontact::sync, for testing i've set them in the main routine. The sensors and door state could potentially be tracked there more efficiently.

Prerequisite for:
#396
#401

@breakingspell breakingspell marked this pull request as draft April 3, 2025 00:16
@breakingspell breakingspell force-pushed the drycontact-limit-check branch from fda9849 to a4d84d1 Compare April 3, 2025 03:29
@breakingspell
Copy link
Contributor Author

breakingspell commented Apr 3, 2025

Hm, sometimes the door can still report CLOSED state if interrupted several times and left halfway mid-close (obstruction sensors, wall button, etc). This is with both sensors off

It probably shouldn't report a definite CLOSED unless the limit sensor is engaged, need to find out where it is being set (I think a timeout is).

@PaulWieland
Copy link
Contributor

PaulWieland commented Apr 3, 2025

I hope I'm understanding the purpose of the checks correctly, but let me know!

The original idea was that a discrete open|close command should not cause the door to move in an unexpected direction. (e.g. open command should never cause the door to close and the close command should never cause the door to open). Thats why discrete commands are ignored if the door state is UNKNOWN requiring the user to either Toggle the door or physically press the wall button.

This is a trade off, but the expectation is that a partially opened door combined with a ratgdo reboot should be a very rare situation.

@breakingspell
Copy link
Contributor Author

breakingspell commented Apr 3, 2025

The original idea was that a discrete open|close command should not cause the door to move in an unexpected direction. (e.g. open command should never cause the door to close and the close command should never cause the door to open).

Hm, I'm not 100% certain, but this doesn't seem to be working as intended in current main:

  • Door is fully closed, Closed sensor on, state reports Closed.
  • In web interface, a discrete command to close cover will "bounce" the door into a fully-open state (I believe the action is converted to Toggle).
  • In HA, the same behavior occurs if you grab the shutter slider, and then release at 0% to "cancel"

With this patch the discrete operations are properly declined by the device when at their limit. I'll test a bit more though.

I also took a look last night at modeling drycontact::sync after the other sec+ protocols to house the sensor and door states, but stashed it for now.

We can also use this MR to address the interrupt/obstruction enhancements noted at #344. I'm not sure right away if my obstruction sensors have the low pulse state, but I see where it's defined in the code so I should be able to follow along.


Semi off-topic, but I had enough reed switches and snap clips left over from the project to set up a smaller harness at my desk. Now I can test these changes without wearing my GDO motor out 😁

@breakingspell breakingspell changed the title Use limit sensors in dry contact door_action() check Use limit sensors for discrete cover operation checks, and report position Apr 3, 2025
@breakingspell breakingspell changed the title Use limit sensors for discrete cover operation checks, and report position Use dry contact limit sensors for discrete cover operation checks, and report position Apr 3, 2025
@breakingspell breakingspell changed the title Use dry contact limit sensors for discrete cover operation checks, and report position Dry Contact sensor/position improvements Apr 3, 2025
@PaulWieland
Copy link
Contributor

In web interface, a discrete command to close cover will "bounce" the door into a fully-open state (I believe the action is converted to Toggle).

A toggle command is used if no obstruction sensors are detected. Most ROW Sec+ 2 openers don't actually have obstruction sensors, and as such they ignore the discrete close command, but accept a toggle command. For that reason the firmware detects if sensors are present, and if not, it issues a toggle instead of a close.

I have to check but its possible that's causing this behavior in the dry contact firmware. We might have to put a check on the protocol before using toggle in lieu of close when no sensors are attached.

@breakingspell
Copy link
Contributor Author

breakingspell commented Apr 3, 2025

A toggle command is used if no obstruction sensors are detected. Most ROW Sec+ 2 openers don't actually have obstruction sensors, and as such they ignore the discrete close command, but accept a toggle command. For that reason the firmware detects if sensors are present, and if not, it issues a toggle instead of a close.

I noticed that behavior when testing away from the actual motor unit (with no obstruction sensors). This remap does properly resolve when the obstruction sensors are present, the controller processes a discrete Close, but the door still cycles when cover position 0.0 is requested.

If it's being incorrectly called while intended for Sec+ 2 though, then perhaps it should be put behind a protocol check, but I noticed those are sparsely used, if at all in the main ratgdo.cpp. There maybe somewhere I'm missing though, thanks for the time to help look!

For full context, my GDO is a Sommer Synoris Direct Drive 550 (1042v004), single-button. The ratgdo has no trouble interfacing with it and detecting obstruction state 👍

@breakingspell
Copy link
Contributor Author

Circling back here, the other dry-contact patches I've drafted are based off this core adjustment to the limit sensor reporting, it should be ready to go on it's own.


I have to check but its possible that's causing this behavior in the dry contact firmware. We might have to put a check on the protocol before using toggle in lieu of close when no sensors are attached.

I double-checked and can confirm that CLOSE is issued properly, as the obstruction sensors are present.

It may still be best to make a protocol distinction rather than checking for existence of a common sensor across protocols, but I don't think it's related to this one, as the correct door action is returned. We can fold that into the PR for the Obstruction sensor improvements.

Also, I only see the preprocessor #ifdef PROTOCOL_DRYCONTACT used once in this manner, and it's stated it's not an ideal case. If it's fine to use that check again, it would work to isolate the behavior to drycontact.


Even with the protocol distinction made for obstruction sensors, these changes to drycontact's limit sensors will:

  • Check the limit sensor state rather than the last known door state before declining an impossible action.

    • The door state could be inaccurate (inverted by obstruction cycle, wall button pressed, reboot while partially open).
    • Prevents the garage door from improperly Opening if DoorAction::CLOSE is issued while already closed.
      • It may be specific to my Sommer GDO, but that scenario will open the door. The opener may be making that call internally, so this ends the invalid action before it can reach the GDO.
  • Set the position float value on sensor trigger

    • This works to "check in" and correct the door state if inaccurate.

All ears for suggestions and feedback!

@breakingspell breakingspell marked this pull request as ready for review May 11, 2025 08:21
@breakingspell breakingspell changed the title Dry Contact sensor/position improvements Dry Contact Limit sensor improvements May 11, 2025
@breakingspell breakingspell force-pushed the drycontact-limit-check branch 2 times, most recently from 819af20 to 477d22c Compare July 14, 2025 22:43
@breakingspell
Copy link
Contributor Author

Hey @bdraco, I noticed the rebase from main, went ahead and squashed the commits here. Thanks!

@breakingspell breakingspell force-pushed the drycontact-limit-check branch 2 times, most recently from a0ea51c to 3ae648b Compare July 14, 2025 23:24
@bdraco
Copy link
Member

bdraco commented Jul 15, 2025

Branches and forks now actually test correctly on the CI. You should be able to update this PR now and the CI will actually test it

@breakingspell
Copy link
Contributor Author

Thanks yes!

It looks like now that it's properly testing the branch, it's failing due to the recent optimizations. Will re-visit, thanks again for fixing the tests

@breakingspell breakingspell force-pushed the drycontact-limit-check branch from ab93124 to cdbcf8a Compare July 15, 2025 05:17
Set door position when limit sensors are triggered
@breakingspell breakingspell force-pushed the drycontact-limit-check branch from cdbcf8a to 8fddca0 Compare July 15, 2025 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants