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

Shelly 2.5 ENERGY_POWER_LIMIT only works for output 1 #6340

Closed
cinnayyannic opened this issue Sep 2, 2019 · 8 comments

Comments

@cinnayyannic
Copy link

@cinnayyannic cinnayyannic commented Sep 2, 2019

TO REPRODUCE

Set a power limit value. Connect load to output 2 of Shelly 2.5 > power limit value

EXPECTED BEHAVIOUR

Output 2 should be turned off if power limit is reached.
I think the problem is that for Shelly 2.5 the power is measured as sum of output 1 and 2.
It would be nice to have a separate power measurement for each output.

If there is power measurement per output the power limit value should be the max power / output. If max power / output is reached the corresponding output should be turned off.

If individual measurement per output is not possible i would expect that at least both outputs are turned off if them sum of both outputs is > power limit value.

@ascillato2

This comment has been minimized.

Copy link
Collaborator

@ascillato2 ascillato2 commented Sep 2, 2019

Closing this issue as it is duplicated from #6160. Sorry.


Support Information (Guide)

See Wiki for more information.
See FAQ for common questions/answers and links if none of your question is in the list
See Chat for more user experience.
See Community for forum.
See Code of Conduct

@cinnayyannic

This comment has been minimized.

Copy link
Author

@cinnayyannic cinnayyannic commented Sep 2, 2019

I dont think it is a duplicate of #6160

The issue is not about dual power metering but about the non functional power limit.
Even without dual power metering i would expect that both outputs are turned off for safety reasons if the power limit is reached.

For over temperature it is done the right way
SetAllPower(POWER_ALL_OFF, SRC_OVERTEMP);

In case of over temperature both outputs are turned off. I think the same should be done for the power limit until dual metering is implemented.

@arendst arendst added enhancement and removed duplicated labels Sep 3, 2019
@arendst

This comment has been minimized.

Copy link
Owner

@arendst arendst commented Sep 3, 2019

Will implement SetAllPower too in near future.

@arendst arendst reopened this Sep 3, 2019
@jziolkowski

This comment has been minimized.

Copy link
Contributor

@jziolkowski jziolkowski commented Sep 3, 2019

Will that command be available to end user, say as POWERALL?

arendst added a commit that referenced this issue Sep 3, 2019
Fix turning on/off all power when limit is reached (#6340)
@arendst arendst added the fixed label Sep 3, 2019
@cinnayyannic

This comment has been minimized.

Copy link
Author

@cinnayyannic cinnayyannic commented Sep 3, 2019

First of all thanks for the fast fix 👍
Now in case of overpower SetAllPower will take care that all channels are turned off.

But i think now the Max power retry acts wrong.
Lets say output 1 was turned on and output 2 was turned off. Output1 is causing the over power situation. Now max power retry will kick in and turn both outputs on. SetAllPower(POWER_ALL_ON, SRC_MAXPOWER);

Also not really a good behaviour. I think the power should be restored to the last known user requested output state.

arendst added a commit that referenced this issue Sep 3, 2019
Add restore power state when limiit restored (#6340)
arendst added a commit that referenced this issue Sep 3, 2019
Add restore power state when limit is restored (#6340)
arendst added a commit that referenced this issue Sep 3, 2019
…t once

Add command Power0 0/1/2/Off/On/Toggle to control all power outputs at once (#6340)
@arendst

This comment has been minimized.

Copy link
Owner

@arendst arendst commented Sep 3, 2019

@jziolkowski just added command power0 0/1/2/Off/On/Toggle to control all outputs at once.

@cinnayyannic

This comment has been minimized.

Copy link
Author

@cinnayyannic cinnayyannic commented Sep 3, 2019

Great should work now unless user changed power state while in over power retry condition.

Just an idea:

sonoff.ino

power_t lastPoweredState = POWER_ALL_OFF

void SetDevicePower(power_t rpower, int source)
{
   ...
      if (count > 1) {
        mask = ~Settings.interlock[i];    // Turn interlocked group off as there would be multiple relays on
        power &= mask;
        rpower &= mask;
      }
    }
  }
   
  ...

  if (rpower != POWER_ALL_OFF)
  {
      lastPoweredState = rpower; // remember last known applyed powered ON state
  }

  ...
}

void RestoreAllPower(int source)
{
  if (power != lastPoweredState) {
    SetDevicePower(lastPoweredState, source);
    MqttPublishAllPowerState();
  }
}

Afterwards just call RestoreAllPower(SOURCE) no need to remember state in xdrv_03_energy.ino or any future source and always latest power ON state.

@arendst

This comment has been minimized.

Copy link
Owner

@arendst arendst commented Sep 4, 2019

Thx. Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.