Skip to content
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

Interlock by software. Controling blinds, curtains or motors #1510

Closed
oscarsan1 opened this issue Jan 27, 2019 · 10 comments · Fixed by #1962
Closed

Interlock by software. Controling blinds, curtains or motors #1510

oscarsan1 opened this issue Jan 27, 2019 · 10 comments · Fixed by #1962
Labels
enhancement New feature or request
Milestone

Comments

@oscarsan1
Copy link

oscarsan1 commented Jan 27, 2019

Going on with issue:
Interlock by software on Sonoff T1 2 gang, or Sonoff Dual
#491

When control curtains or motors with T1 2 gang I'm afraid of this ms that two realys are on. Is safer to take some time to switch off before switch on the new relay. This time is also good for the motor to stop moving before start in the other direction. Also relays offen takes more time to switch off (15ms) than to switch on (5 ms).
I propose to modify "void relaySync(unsigned char id)" in the case of RELAY_SYNC_ANY

    if (relaySync != RELAY_SYNC_ANY)
    {
        for (unsigned short i = 0; i < _relays.size(); i++)
        {
            if (i != id)
            {
                if   ( relayStatus(i) ){  // if relay i is on
                           relayStatus(i, false);
                                           //delay 500 ms before on for wait to turn off the other relay
                           _relays[id].change_time = millis() + 500; 
               }
            }
        }
    }

This will work?

@oscarsan1 oscarsan1 added the enhancement New feature or request label Jan 27, 2019
@oscarsan1 oscarsan1 changed the title Interlock by Controling blinds, curtains or motors Interlock by software. Controling blinds, curtains or motors Jan 27, 2019
@oscarsan1
Copy link
Author

oscarsan1 commented Feb 8, 2019

Finaly I can test it, fixed a bug in last proposal. I test this one and works well. Only one relay on and at lest 500 ms between relay "x" off an relay "y" on:

void relaySync(unsigned char id){
...
       // If NONE_OR_ONE or ONE and setting ON we should set OFF all the others
    }
    else if (status)
    {
        if (relaySync != RELAY_SYNC_ANY)
        {
            for (unsigned short i = 0; i < _relays.size(); i++)
            {
                if (i != id){
                    if   ( relayStatus(i) ){  // if current status relay i is on
                        _relays[id].change_time = millis() + 500; //delay 500 ms before on for wait to turn off the other relay
                    }
                    relayStatus(i, false);  
               }
            }
        }

        // If ONLY_ONE and setting OFF we should set ON the other one
    }

@xoseperez
Copy link
Owner

relayStatus does not change the relay status right away, it only enqueues the change. It the _relayProcess method that processes the queue and calls _relayProviderStatus that executes the changes. The _relayProcess is first called to turn whatever has to be turned off and the whatever has to be turned on. I think it should be in between this two call where a safety delay should be added, probably in the _relayLoop method that does both calls.

@oscarsan1
Copy link
Author

oscarsan1 commented Feb 10, 2019

Yes I see now.
But the interlock have to enqueues the "on" order delaying 500 ms. I only modify the original place for "none or one relay on" adding delay of 500 ms in relaySync(...) function. You're right than it should be safer that it should be in _relayProcess but I think that is not so simply that in realySync (...)

Finaly I upgrade the proposal controling the case of FloodWindow delay "off", that could makes a two relay on.

`        if (relaySync != RELAY_SYNC_ANY)
        {
            for (unsigned short i = 0; i < _relays.size(); i++)
            {
                if (i != id){
                    if   ( relayStatus(i) ){  // if current status relay i is on
                        unsigned long current_time = millis();
                        _relays[id].change_time = current_time + 500; //delay 500 ms before on for wait to turn off the other relay
                                   
                        _relays[i].fw_start = current_time;  // We reset the floodWindow relay i, as turning off is prioritary and can't wait flood window time
                        _relays[i].fw_count = 1;
                    }
                    relayStatus(i, false);  
               }
            }
        }`

I test a lot in sonoff T1 2ch without problem.

@xoseperez xoseperez added this to the 1.14.0 milestone Feb 10, 2019
@oscarsan1
Copy link
Author

oscarsan1 commented Feb 16, 2019

Also, I wan't to ensure that no change in the sync mode occurs accidentaly, via web for example. I haven't fount a way to disable it in the setting or via flags, so I add to my code:

`    byte relaySync = getSetting("relaySync", RELAY_SYNC).toInt();
    bool status = _relays[id].target_status;

#if RELAY_SYNC == RELAY_SYNC_NONE_OR_ONE   // new part
    if (relaySync!=RELAY_SYNC_NONE_OR_ONE){
        relaySync=RELAY_SYNC_NONE_OR_ONE;
        setSetting("relaySync", RELAY_SYNC_NONE_OR_ONE);
    }
#endif

    // If RELAY_SYNC_SAME all relays should have the same state`

Then every time relay must chage verify if the RELAY_SYNC was initialy configured as NONE_OR_ONE, eventhougt it was changed via Web or other, reassing the original NONE_OR_ONE and save this Setting.

I create two new env. for blinds:

[env:itead-sonoff-t1-2ch-blinds]
platform = ${common.platform}
framework = ${common.framework}
board = ${common.board_1m}
board_build.flash_mode = ${common.flash_mode}
lib_deps = ${common.lib_deps}
lib_ignore = ${common.lib_ignore}
build_flags = ${common.build_flags_1m0m} -DITEAD_SONOFF_T1_2CH 
    -DRELAY_SYNC=RELAY_SYNC_NONE_OR_ONE -DRELAY_PULSE_MODE=RELAY_PULSE_OFF -DRELAY_PULSE_TIME=25
monitor_speed = ${common.monitor_speed}
extra_scripts = ${common.extra_scripts}

[env:itead-sonoff-t1-2ch-ota-blinds]
platform = ${common.platform}
framework = ${common.framework}
board = ${common.board_1m}
board_build.flash_mode = ${common.flash_mode}
lib_deps = ${common.lib_deps}
lib_ignore = ${common.lib_ignore}
build_flags = ${common.build_flags_1m0m} -DITEAD_SONOFF_T1_2CH 
    -DRELAY_SYNC=RELAY_SYNC_NONE_OR_ONE -DRELAY_PULSE_MODE=RELAY_PULSE_OFF -DRELAY_PULSE_TIME=25
upload_speed = ${common.upload_speed}
upload_port = ${common.upload_port}
upload_flags = ${common.upload_flags}
monitor_speed = ${common.monitor_speed}
extra_scripts = ${common.extra_scripts}

@ctandi
Copy link

ctandi commented Mar 5, 2019

@xoseperez Has anything happened regarding this issue?

@xoseperez
Copy link
Owner

No, I'm planning to add it to dev ASAP. Maybe adding also the possibility to define the interlock delay in the web interface...

@xoseperez xoseperez modified the milestones: 1.14.0, 1.13.6 Mar 20, 2019
@mcspr
Copy link
Collaborator

mcspr commented Oct 25, 2019

Sort-of on topic. This line bothered me for some time, since "49 Days Later" current_time becomes 0. Logic-wise, it delays a bit and things sort of work anyways, but I think the struct needs a different way to manage delays:

if (current_time < _relays[id].fw_start || fw_end <= current_time) {

This should be fixed by storing current_time on the struct instead of required change_time and comparing applied delay (ON, OFF, flood) as would any other timestam would be compared:

millis() - change_start > change_delay

ATM I am just playing with this approach, but it should be trivial to add additional delays for sync mode. As I understood it, the thing we want is to make relays switch sequentially instead of operating on a single timer for ON / OFF.

@mcspr
Copy link
Collaborator

mcspr commented Oct 28, 2019

@oscarsan1 looking at your requirements again, wouldn't just adding DELAY_ON=500 for each relay force this to happen?

Per #1962 (will push update, nvm current offsets for change_delay when syncing), only issue is uncertainty that loop() timeouts for synchronous ntp, mqtt etc. will force both to happen simultaneously when relay loop function finally starts again. Rough solution is to recalculate change_start time after relayProcess function finishes, making delays relative to each other instead of depending on when relayStatus(N, true) + relaySync had happened.

Another problem is that while we only want this delay to happen if any other relay is ON, user probably expects no delay when both are off. In case of sub-second delays, perhaps it is overly precious, but #161 (comment) noted one of such use-cases

@oscarsan1
Copy link
Author

Hi.
As you mention the delay_on must happen only if another relay are on, in order to wait until this on relay switch to off before the new relay switch to on. If this delay_on happens always, user gets delayed actions every pushed button and other actions, that don't need this delay.
I review your code but don't understand all of it.
In my proposal I think that if there is another action like mqtt for swiching relay on while wating the 500 ms on a previous on order, the previous order was canceled and the new one mqtt was delayed 500 ms. isn't it?
I try to test your code in the next days in my blinds and report the result.

@mcspr
Copy link
Collaborator

mcspr commented Oct 30, 2019

If you have comments regarding the PR, feel free to use inline review tool right there in the code (see blue plus sign near the line numbers, when hovering over it)

And yes, right now delay time will be updated after each relayStatus call if target status stays the same or cancelled if current_status == status of the function. e.g. you can send ON via mqtt, delay on kicks in for 10000 ms as an example, and while it happens you can send OFF to cancel.
One quirk though. In the examples above, when the other relay that is turning OFF because of relaySync it will not be scheduled to turn back ON again, so this approach does not have full rollback behaviour.

So does this mean we need to have a single constant delay value? Or should delayOn and delayOff change meaning depending on sync mode?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants