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

Switcher power plug with firmware >= 1.46 supports timed base capabilities #335

Open
TomerFi opened this issue Aug 26, 2021 · 16 comments · May be fixed by #833
Open

Switcher power plug with firmware >= 1.46 supports timed base capabilities #335

TomerFi opened this issue Aug 26, 2021 · 16 comments · May be fixed by #833
Labels
status: confirmed Issue confirmed type: enhancement New feature or request

Comments

@TomerFi
Copy link
Owner

TomerFi commented Aug 26, 2021

What did you had in mind?

According to research performed by @thecode and @dmatik,
Switcher power plugs with firmware >= 1.46 supports timed based capabilities such as:

  • Auto shutdown of the device based on the configured value.
  • Displaying the remaining time for the current run.

These are capabilities that up until now were used in Switcher water heater devices.
It should be stated, power plugs with firmware < 1.46, return a default value for the remaining time,
So any modification made for this enhancement, should not affect backward compatibility.

Are you trying to fix a problem?

Power plugs with the firmware of >= 1.46 come with the configuration option for auto-shutdown.
This configuration is not visible to this integration.

Any lead on how this feature can be implemented?

We can split this into two workflows:

  1. Setting the device configuration auto-shutdown value and/or turning it on with a designated timer (timer is not supported see comment,) should probably work out of the box, as the API context doesn't verify the type of the device before sending out the required packet. So I don't think any modifications are required for this part.

  2. The device state should display the new capabilities value.
    This will require some modifications, so let's dive in:

  • The DeviceCategory Enum is used to categorized the device type.
  • The DeviceType Enum helps us identify the discovered device.
  • The DeviceState Enum although, not quite related to this issue, but worth mentioning, helps us categorized the device's state.

  • The SwitcherBase class is the abstraction all switcher devices inherit from.
  • The SwitcherPowerBase class is an abstraction used for switcher devices with the capability to have power_consumption and electric_current properties.
  • The SwitcherTimedBase class is an abstraction used for switcher devices with the capability to have remaining_time and auto_shutdown properties.

  • The SwitcherPowerPlug class is a concrete implementation of the switcher power plug, it inherits from both SwitcherPowerBase and SwitcherBase.
  • The SwitcherWaterHeater class is a concrete implementation of the switcher water heater, it inherits from SwitcherTimedBased, SwitcherPoerBase, and SwitcherBase.

  • The _parse_device_from_datagram private method is the one place that identifies the different device types.
  • The SwitcherBridge is the udp context manager that utilizes the above _parse_device_from_datagram in order to return the correct device.

Although, as for this enhancement, the power plugs and water heater are basically the same devices.
I wouldn't recommend dropping the separation between them,
we can just add capabilities to the power plug representation and make it similar to the water heater one.


In order to give the power plug timed-based capabilities, we simply need to make the SwitcherPowerPlug class inherit from SwitcherTimedBased as well as the current inheritance from SwitcherPowerBase and SwitcherBase.
Just remember - it's python - order matters so keep it in line with the SwitcherWaterHeater class.

We then need to add the two new properties when discovering a power plug device,
in _parse_device_from_datagram,
find the power plug if branch and add the new properties,
it should be quite similar to the water heater if branch.

These modifications are allegedly not that big or complicated,
But I assume testing modification will be required as well.

@TomerFi TomerFi added the type: enhancement New feature or request label Aug 26, 2021
@github-actions github-actions bot added the status: needs triage Issue needs manual triage label Aug 26, 2021
@TomerFi TomerFi added status: confirmed Issue confirmed status: on-hold This issue or pull request is on hold and removed status: needs triage Issue needs manual triage labels Aug 26, 2021
@thecode thecode added type: wip Work in progress and removed status: on-hold This issue or pull request is on hold labels Jan 7, 2022
@thecode
Copy link
Collaborator

thecode commented Jan 7, 2022

I started working on this, first thing I noticed is that Switcher app only let you set the auto_shutdown value and does not have in the UI an option to turn on the plug with a timer.
I have made the changes required to report the auto_shutdown and remaining_time and they show correct values, however as the app hints, device does not support turning on with timer, I have validated that we send a correct packet. The timer is just ignored by the device (e.g. it will operate using the auto_shutdown value.

The good thing is that it has no impact on the plan above. as the changes are only made to the paring of incoming status (2. above) and we don't to any changes to the device control (1. above) this will work as planned.

The user for the library should not allow setting a timer, although device will only ignore the value and still perform the turn on request.
For Home Assistant there is already a warning if you try to set a timer value or set the auto_shutdown for plugs so I only plan to remove the warning for auto_shutdown.

The fact that they are not exactly identical only makes it more clear that we should keep the separation between these two devices.

I will edit the comment to indicate the missing timer option.

@thecode
Copy link
Collaborator

thecode commented Jan 9, 2022

Additional information;
Switcher plug supports disabling auto shutdown, in the up it is displayed as unlimited. When the auto shutdown to disabled the device reports a value of 108000 seconds (30 hours) in the auto_shutdown and remaining_time fields. Currently our code is limited to <24 hours.
We will need to discuss how to add this option, something like allowing setting a value of "disabled" (as a string) and reporting the same or keeping the API / status messages with the original value (which will show as 30:00:00) and let the higher level library user to convert the values to user friendly fields.

@TomerFi
Copy link
Owner Author

TomerFi commented Jan 14, 2022

This is very good progress, sorry it took me so long to respond.
The < 24-hour limit is per the limitation of the water heater I believe.
If that's the case, we may need to verify the device type before enforcing the limitation,
that is presuming, of course, that both the water heater and the power plug will use the same API infrastructure for relaying the auto-shutdown configuration value.

@TomerFi
Copy link
Owner Author

TomerFi commented Jan 14, 2022

This is the utility function that handles the conversion of the timedelta object to seconds and then to a hex representation.
Currently, it is also in charge of enforcing the > 24-hour limit, maybe while incorporating this ability to power plugs too, which as you discovered, has a different limitation (30-hour) we need to refactor and remove the enforcement logic from this function.

It is invoked by this API function, which is called directly by the customer via the context manager, maybe it will be better to put the enforcement logic here, so we can verify differently per device and call the utility function just for the conversion.

Currently, as seen here, the > 24-hour limitation is enforced after the conversion from timedelta to seconds.
We can remove this part, verify the timedelta itself here and raise the appropriate value error per limitation per device type, it will probably be a better idea to do this regardless of the power plug modifications.

@TomerFi
Copy link
Owner Author

TomerFi commented Jan 15, 2022

I was scratching my head all night thinking about how can make this work.
I wanted to suggest specifying the auto-shutdown limit as a field in the DeviceCategory or DeviceType enums, but the API is so loosely coupled that it doesn't know, nor does it needs to know the device type or category before sending the request.

I see now what you meant by "We will need to discuss how to add this option", this is gonna be a tricky addition to this codebase.
🤔

@thecode thecode removed the type: wip Work in progress label Jun 5, 2022
@TomerFi TomerFi added the hacktoberfest hacktoberfest label Sep 23, 2022
@YogevBokobza
Copy link
Collaborator

@thecode @TomerFi
In the current library implementation can we work on this issue?
Can you summarize what needs to be done?

@TomerFi
Copy link
Owner Author

TomerFi commented Dec 23, 2024

@YogevBokobza

API

Control device

We need to determine whether any modifications are required for the set_auto_shutdown function. Does it work for power plugs like it works for water heaters?

Get state

The get_state function, returns a SwitcherStateResponse, which uses the StateMessageParser for parsing the message. Will the get_auto_shutdown function work?

Bridge

Update device implementation

The SwitcherWaterHeater device class implements:

The SwitcherPowerPlug device class, on the other hand, only implements SwitcherPowerBase and SwitcherBase.
According to this issue, starting firmware >= 1.46, SwitcherPowerPlug can also implement SwitcherTimedBase.

This will also mean SwitcherWaterHeater and _SwitcherPowerPlug_are identical; should we merge them?

Parse device state

After updating the device implementation, the DatagramParser must also be modified. For the SwitcherPowerPlug case, a remaining_time and an auto_shutdown must be passed as well, much like we do for the SwitcherWaterHeater case.

Will the get_remaining and get_auto_shutdown work the same for water heaters and power plug?

Value overlap

Auto shutdown

As noted by @thecode here, there's a different behaviour of the value we get for auto_shutdown from water heaters and power plugs. The water heater limits this value to 24h, while power plugs use 30h to indicate that the configuration is off. We need to explore these contrasts and understand where they meet us. Do we need to take this under consideration in the API's set_auto_shutdown function, the API's state get_auto_shutdown function, the bridge's get_auto_shutdown function, or all of the above?

@TomerFi TomerFi removed the hacktoberfest hacktoberfest label Dec 28, 2024
@YogevBokobza YogevBokobza linked a pull request Jan 16, 2025 that will close this issue
2 tasks
@YogevBokobza
Copy link
Collaborator

YogevBokobza commented Jan 16, 2025

So I tested this issue and here is what I learned:

Regarding

We need to determine whether any modifications are required for the set_auto_shutdown function. Does it work for power plugs like it works for water heaters?

It seems like no modifications are needed.
tested working: poetry run control_device set_auto_shutdown -c "Switcher Power Plug" -d 47fc1f -i "192.168.1.186" -r 1 -m 0 -v

Regarding

The get_state function, returns a SwitcherStateResponse, which uses the StateMessageParser for parsing the message. Will the get_auto_shutdown function work?

It seems like no modifications are needed.

Regarding

Update device implementation
The SwitcherWaterHeater device class implements:
SwitcherTimedBase (remaining_time, auto_shutdown).
SwitcherPowerBase (power_consumption, electric_current).
SwitcherBase (device_type, device_state, device_id, etc).
The SwitcherPowerPlug device class, on the other hand, only implements SwitcherPowerBase and SwitcherBase.
According to this issue, starting firmware >= 1.46, SwitcherPowerPlug can also implement SwitcherTimedBase.
This will also mean SwitcherWaterHeater and _SwitcherPowerPlug_are identical; should we merge them?

Let me know if you want to merge those APIs or not (Will be modified in #833)

Regarding

After updating the device implementation, the DatagramParser must also be modified. For the SwitcherPowerPlug case, a remaining_time and an auto_shutdown must be passed as well, much like we do for the SwitcherWaterHeater case.
Will the get_remaining and get_auto_shutdown work the same for water heaters and power plug?

Yes get_remaining and get_auto_shutdown work the same.

Regarding

As noted by @thecode #335 (comment), there's a different behaviour of the value we get for auto_shutdown from water heaters and power plugs. The water heater limits this value to 24h, while power plugs use 30h to indicate that the configuration is off. We need to explore these contrasts and understand where they meet us. Do we need to take this under consideration in the API's set_auto_shutdown function, the API's state get_auto_shutdown function, the bridge's get_auto_shutdown function, or all of the above?

Yes here we have some differences and I fix that by:
https://github.com/TomerFi/aioswitcher/pull/833/files#diff-76fda1abe206ffc78578e59e694e42cf998f9158ec5427d72e3f42d8db31c7a4R47-R50 - This will reset the time for "00:00:00" if the hours >24 (which mean that the configuration is off)
Also, this https://github.com/TomerFi/aioswitcher/pull/833/files#diff-64dc5b5da1f92f50384c952409ea7e112b7a302b1c526e25add2819a36d07ab9R139-R142 fix the case when the configuration is off and the power plug is on so no remaining will be shown (as should be)

@YogevBokobza
Copy link
Collaborator

Small update:
After applying this:

parser.get_remaining()
if device_state == DeviceState.ON
and parser.get_auto_shutdown() != "00:00:00"
else "00:00:00"

Looks like we no longer need this

Before the above I got higher than 23 in hours that's why I had to reset (h,m,s) but it seems like it was happening only when the function got called by get_remaining and not by get_auto_shutdown function.

@thecode
Copy link
Collaborator

thecode commented Jan 16, 2025

Did you try to set unlimited time in the Switcher app? my plug returned 30:00:00 when setting unlimited and not 00:00:00 as you based your changes on

@YogevBokobza
Copy link
Collaborator

Did you try to set unlimited time in the Switcher app? my plug returned 30:00:00 when setting unlimited and not 00:00:00 as you based your changes on

If by 'unlimited' you mean turning off the "auto-shutdown" button then yes.
You can't set "00" for hours in the Switcher App, it starts from "01".

@YogevBokobza
Copy link
Collaborator

I did notice some kind of odd behabvior:

  1. Set auto shutdown for 01:01:00
  2. Start device
  3. Running get_state API and current auto shut down and remaining time displayed
  4. I changed in Switcher app auto shutdown for 02:00:00
  5. The remaining time did not get updated
  6. When I switched off and off the device the remaining time did get updated

Looks like its coming like this from the packet..

@thecode
Copy link
Collaborator

thecode commented Jan 16, 2025

Did you try to set unlimited time in the Switcher app? my plug returned 30:00:00 when setting unlimited and not 00:00:00 as you based your changes on

If by 'unlimited' you mean turning off the "auto-shutdown" button then yes. You can't set "00" for hours in the Switcher App, it starts from "01".

We are having a broken discussion, when I set in the past (I don't have a working plug now, will need to find one and check again) "unlimited" time for the plug the value returned by the device was 30:00:00 for the remaining time and not 00:00:00, the same value was also used for the API call to disable auto-shutdown, see #335 (comment)
Switcher will show it as 00:00:00 in the app but our methods will break.
Did you try to disable auto shutdown on a plug using our API and test it?

@YogevBokobza
Copy link
Collaborator

Did you try to set unlimited time in the Switcher app? my plug returned 30:00:00 when setting unlimited and not 00:00:00 as you based your changes on

If by 'unlimited' you mean turning off the "auto-shutdown" button then yes. You can't set "00" for hours in the Switcher App, it starts from "01".

We are having a broken discussion, when I set in the past (I don't have a working plug now, will need to find one and check again) "unlimited" time for the plug the value returned by the device was 30:00:00 for the remaining time and not 00:00:00, the same value was also used for the API call to disable auto-shutdown, see #335 (comment) Switcher will show it as 00:00:00 in the app but our methods will break. Did you try to disable auto shutdown on a plug using our API and test it?

I don't know what happened in the past but yes I tested this with my power plug device and its working
this and this make the behavior identical to heater plug

@TomerFi
Copy link
Owner Author

TomerFi commented Jan 17, 2025

Let me know if you want to merge those APIs or not (Will be modified in #833)

Let's hold with the merge, we can live with two identical classes (SwitcherWaterHeater and SwitcherPowerPlug) for a while, and merge them later on as part of a refactoring effort.

@thecode
Copy link
Collaborator

thecode commented Jan 30, 2025

@YogevBokobza since we started the discussion here I am adding the results here and not on the linked PR. This is with testing with my Switcher Plug:

Exception in callback _ProactorDatagramTransport._loop_reading(<_OverlappedF...186', 49154))>)
handle: <Handle _ProactorDatagramTransport._loop_reading(<_OverlappedF...186', 49154))>)>
Traceback (most recent call last):
  File "C:\Program Files\Python312\Lib\asyncio\events.py", line 88, in _run
    self._context.run(self._callback, *self._args)
  File "C:\Program Files\Python312\Lib\asyncio\proactor_events.py", line 589, in _loop_reading
    self._protocol.datagram_received(data, addr)
  File "C:\Data\GitHub\aioswitcher\.venv\Lib\site-packages\aioswitcher\bridge.py", line 451, in datagram_received
    self._on_datagram(data)
  File "C:\Data\GitHub\aioswitcher\.venv\Lib\site-packages\aioswitcher\bridge.py", line 144, in _parse_device_from_datagram
    parser.get_auto_shutdown(),
    ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Data\GitHub\aioswitcher\.venv\Lib\site-packages\aioswitcher\bridge.py", line 564, in get_auto_shutdown
    return seconds_to_iso_time(int_auto_shutdown_val_secs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Data\GitHub\aioswitcher\.venv\Lib\site-packages\aioswitcher\device\tools.py", line 48, in seconds_to_iso_time
    return datetime.time(hour=hours, minute=minutes, second=seconds).isoformat()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: hour must be in 0..23

To debug I added a debug print before

return datetime.time(hour=hours, minute=minutes, second=seconds).isoformat()
and the output is 30 hours as I explained in my previous comments.
hours: 30, minutes: 0, seconds: 0

My plug is at version 1.46 (which is the most common version from users reports),

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

Successfully merging a pull request may close this issue.

3 participants