Add #558, #505: Feature/ Hub Siren and C420 cameras fixes

[marcosngomezi]

Related to pytapo/issues/105 Addresses https://github.com/JurajNyiri/HomeAssistant-Tapo-Control/issues/558, https://github.com/JurajNyiri/HomeAssistant-Tapo-Control/issues/505 Requires pytapo/pull/106

Changes:

• Refactored alarm_config in camData
• Multi devices support of siren
  • Backup command for Siren start and stop, since startManualAlarm and stopManualAlarm are not working for C420 cameras
  • Time_left used on devices returning it
• New TapoHubSirenDuration and TapoHubVolume numbers
• New TapoHubSirenType select

[JurajNyiri]

Hi @marcosngomezi

First, thank you for the great work here!

Could you please test this branch https://github.com/JurajNyiri/pytapo/tree/experiment-multiple-params-getSirenTypeList

My cameras do not support this function at all so I cannot test.

What is the response you get when you run print(tapo.getMost()["getSirenTypeList"])? and what is the response when you run getHubSirenTypeList(self) and getAlarmConfig(self)?

[marcosngomezi]

Can't do the test right now, but already tested to send both msg_alarm and siren on a command and ignores the second one or fails depending on the method, I will test it with your branch and report back

[marcosngomezi]

I'm back @JurajNyiri Sent to both devices hub and camera each command: And the response for each was: As commented on the first image getHubSirenTypeList on the camera throws an exception not sure why that error generates an exception while other don't.

As you can see the camera answers more similar to the rest of the ip cameras, is the hub the one that needs different commands.

[marcosngomezi]

This PR is not solving #556, gives the same functionality but only for hub siren, not camera, I can look at it and try to solve it in another PR It is solving #505 with the connectionInformation fix

[JurajNyiri]

So it looks like when getSirenTypeList is used inside getMost with both arguments {"msg_alarm": {}, "siren": {}}, it does not work on a hub, correct? Maybe we could still put it there as a separate entry instead, one with msg_alarm and another one with siren and then identify the correct response... I will think more about it tomorrow.

[marcosngomezi]

Is what I tried to do on the first commits of the pytapo PR with method aliases, but because the response order is not the same as the request, and doesn't says if its for siren o msg_alarm, I didnt found a trustable way to do it. Maybe a second multiple request inside getMost, but I think that is not possible on the same one.

[JurajNyiri]

@marcosngomezi thank you for testing, I pushed a new commit in the branch https://github.com/JurajNyiri/pytapo/tree/experiment-multiple-params-getSirenTypeList .

Could you please run tapo.getMost() both on camera and hub and post output? No need to put it inside print as this commit is targetting a new approach which should allow us to pair request to response and therefore run the same function mutliple times.

You should see something like

{'request': {'method': 'getConnectionType', 'params': {'network': {'get_connection_type': []}}}, 'response': {'result': {'link_type': 'wifi', 'ssid': 'redacted', 'rssiValue': -46, 'rssi': '4'}, 'method': 'getConnectionType'}}
{'request': {'method': 'getAlarmConfig', 'params': {'msg_alarm': {}}}, 'response': {'result': False, 'method': 'getAlarmConfig'}}
{'request': {'method': 'getAlarmPlan', 'params': {'msg_alarm_plan': {}}}, 'response': {'result': False, 'method': 'getAlarmPlan'}}
{'request': {'method': 'getSirenTypeList', 'params': {'msg_alarm': {}}}, 'response': {'result': False, 'method': 'getSirenTypeList'}}
{'request': {'method': 'getSirenTypeList', 'params': {'siren': {}}}, 'response': {'result': False, 'method': 'getSirenTypeList'}}
{'request': {'method': 'getSirenConfig', 'params': {'siren': {}}}, 'response': {'result': False, 'method': 'getSirenConfig'}}
{'request': {'method': 'getLightTypeList', 'params': {'msg_alarm': {}}}, 'response': {'result': False, 'method': 'getLightTypeList'}}
{'request': {'method': 'getSirenStatus', 'params': {'msg_alarm': {}}}, 'response': {'result': False, 'method': 'getSirenStatus'}}
{'request': {'method': 'getSirenStatus', 'params': {'siren': {}}}, 'response': {'result': False, 'method': 'getSirenStatus'}}

Of course with different values.

Please let me also know if the values are as expected compared to when you call the functions (getHubSirenTypeList, getHubSirenConfig, getHubSirenStatus) directly as in your pr.

[JurajNyiri]

I have pushed another PR which should get this to very near prod quality. @marcosngomezi please grab https://github.com/JurajNyiri/pytapo/tree/experiment-multiple-params-getSirenTypeList and run tapo.getMost() on both camera and hub.

Now you should get output like

getConnectionType
{'link_type': 'wifi', 'ssid': 'redacted', 'rssiValue': -45, 'rssi': '4'}
getAlarmConfig
False
getAlarmPlan
False
getSirenTypeList
[False, False]
getSirenConfig
False
getLightTypeList
False
getSirenStatus
[False, False]

Of course with different values.

Please let me also know if the values are as expected compared to when you call the functions (getHubSirenTypeList, getHubSirenConfig, getHubSirenStatus) directly as in your pr.

[marcosngomezi]

I'm getting the out of order exception, but I think that I get what you are trying to do, and there should be no need to depend on the order, taking the method from the result and just returning the one (or first) that is not False

[JurajNyiri]

That is interesting indeed! I do not get out of order exception. Does that happen only on hub or camera as well? That would block this approach unfortunately.

[marcosngomezi]

Just pushed a possible solution without depending on order at all PR, it works for me:

Hub getConnectionType {'link_type': 'ethernet'}
Hub getAlarmConfig False
Hub getAlarmPlan False
Hub getSirenTypeList {'siren_type_list': ['Doorbell Ring 1', 'Doorbell Ring 2', 'Doorbell Ring 3', 'Doorbell Ring 4', 'Doorbell Ring 5', 'Doorbell Ring 6', 'Doorbell Ring 7', 'Doorbell Ring 8', 'Doorbell Ring 9', 'Doorbell Ring 10', 'Phone Ring', 'Alarm 1', 'Alarm 2', 'Alarm 3', 'Alarm 4', 'Dripping Tap', 'Alarm 5', 'Connection 1', 'Connection 2']}
Hub getSirenConfig {'siren_type': 'Alarm 3', 'volume': '7', 'duration': 10}
Hub getLightTypeList False
Hub getSirenStatus {'status': 'off', 'time_left': 0}
Camera getConnectionType False
Camera getAlarmConfig {'enabled': 'off', 'alarm_mode': ['siren'], 'siren_type': 'Siren', 'light_type': 'flicker', 'siren_volume': 'high', 'siren_duration': 5}
Camera getAlarmPlan {'enabled': 'off', 'alarm_plan': '0000-0000,127'}
Camera getSirenTypeList {'siren_type_list': ['Siren', 'Tone', 'Red Alert']}
Camera getSirenConfig False
Camera getLightTypeList {'light_type_list': ['flicker']}
Camera getSirenStatus {'status': 'off'}
Get Hub Siren Type List
Hub {'siren_type_list': ['Doorbell Ring 1', 'Doorbell Ring 2', 'Doorbell Ring 3', 'Doorbell Ring 4', 'Doorbell Ring 5', 'Doorbell Ring 6', 'Doorbell Ring 7', 'Doorbell Ring 8', 'Doorbell Ring 9', 'Doorbell Ring 10', 'Phone Ring', 'Alarm 1', 'Alarm 2', 'Alarm 3', 'Alarm 4', 'Dripping Tap', 'Alarm 5', 'Connection 1', 'Connection 2']}

Get Hub Siren Status
Hub {'status': 'off', 'time_left': 0} 

Get Hub Siren Config
Hub {'siren_type': 'Alarm 3', 'volume': '7', 'duration': 10} 

Get Alarm config
Hub [{'method': 'getAlarmPlan', 'error_code': -40106}, {'method': 'getAlarmConfig', 'error_code': -40106}, {'method': 'getLightTypeList', 'error_code': -40106}, {'method': 'getSirenTypeList', 'error_code': -40106}, {'method': 'getSirenStatus', 'error_code': -40106}]
Camera [{'method': 'getAlarmConfig', 'result': {'enabled': 'off', 'alarm_mode': ['siren'], 'siren_type': 'Siren', 'light_type': 'flicker', 'siren_volume': 'high', 'siren_duration': 5}, 'error_code': 0}, {'method': 'getAlarmPlan', 'result': {'enabled': 'off', 'alarm_plan': '0000-0000,127'}, 'error_code': 0}, {'method': 'getSirenTypeList', 'result': {'siren_type_list': ['Siren', 'Tone', 'Red Alert']}, 'error_code': 0}, {'method': 'getLightTypeList', 'result': {'light_type_list': ['flicker']}, 'error_code': 0}, {'method': 'getSirenStatus', 'result': {'status': 'off'}, 'error_code': 0}]

The problem to solve now is for the sets how to know if should be siren or msg_alarm or send both? getConnectionType is not going to be useful because both ip cameras and hub respond to it, maybe with getSirenConfig and getAlarmConfig?

[marcosngomezi]

Ah just realised you where trying to make a list with the multiple results, instead to just keep one, what do you think about my approach? As I said the problem is that you don't know which method was the successful and need to depend on another way to know which values should use for sets

[marcosngomezi]

Looks like only the hub has the order problem, but the camera has a lot of missing result methods: Hub request - result:

getDeviceInfo getDetectionConfig
getDetectionConfig getVehicleDetectionConfig
getPersonDetectionConfig getPersonDetectionConfig
getVehicleDetectionConfig getBarkDetectionConfig
getBCDConfig getMeowDetectionConfig
getPetDetectionConfig getBCDConfig
getBarkDetectionConfig getPetDetectionConfig
getMeowDetectionConfig getDeviceInfo
getGlassDetectionConfig getGlassDetectionConfig
getTamperDetectionConfig getTamperDetectionConfig
getLensMaskConfig getLensMaskConfig
getLdc getLdc
getLastAlarmInfo getLastAlarmInfo
getLedStatus getLedStatus
getTargetTrackConfig getTargetTrackConfig
getPresetConfig getPresetConfig
getFirmwareUpdateStatus getFirmwareUpdateStatus
getMediaEncrypt getMediaEncrypt
getConnectionType getConnectionType
getAlarmConfig getAlarmConfig
getAlarmPlan getAlarmPlan
getSirenTypeList getSirenTypeList
getSirenTypeList getSirenTypeList
getSirenConfig getSirenConfig
getLightTypeList getLightTypeList
getSirenStatus getSirenStatus
getSirenStatus getSirenStatus
getLightFrequencyInfo getLightFrequencyInfo
getLightFrequencyCapability getLightFrequencyCapability
getChildDeviceList getRotationStatus
getRotationStatus getNightVisionModeConfig
getNightVisionModeConfig getWhitelampStatus
getWhitelampStatus getWhitelampConfig
getWhitelampConfig getMsgPushConfig
getMsgPushConfig getSdCardStatus
getSdCardStatus getCircularRecordingConfig
getCircularRecordingConfig getRecordPlan
getRecordPlan getAudioConfig
getAudioConfig getFirmwareAutoUpgradeConfig
getFirmwareAutoUpgradeConfig getChildDeviceList

Camera:

getDeviceInfo getDeviceInfo
getDetectionConfig getDetectionConfig
getPersonDetectionConfig getPersonDetectionConfig
getVehicleDetectionConfig getVehicleDetectionConfig
getBCDConfig
getPetDetectionConfig getPetDetectionConfig
getBarkDetectionConfig
getMeowDetectionConfig
getGlassDetectionConfig
getTamperDetectionConfig
getLensMaskConfig getLensMaskConfig
getLdc getLdc
getLastAlarmInfo
getLedStatus getLedStatus
getTargetTrackConfig
getPresetConfig
getFirmwareUpdateStatus getFirmwareUpdateStatus
getMediaEncrypt getMediaEncrypt
getConnectionType
getAlarmConfig getAlarmConfig
getAlarmPlan getAlarmPlan
getSirenTypeList getSirenTypeList
getSirenTypeList
getSirenConfig
getLightTypeList getLightTypeList
getSirenStatus getSirenStatus
getSirenStatus
getLightFrequencyInfo getLightFrequencyInfo
getLightFrequencyCapability getLightFrequencyCapability
getChildDeviceList
getRotationStatus getRotationStatus
getNightVisionModeConfig getNightVisionModeConfig
getWhitelampStatus getWhitelampStatus
getWhitelampConfig getWhitelampConfig
getMsgPushConfig getMsgPushConfig
getSdCardStatus getSdCardStatus
getCircularRecordingConfig getCircularRecordingConfig
getRecordPlan getRecordPlan
getAudioConfig getAudioConfig
getFirmwareAutoUpgradeConfig

Hub response is also not deterministic with the order, it looks like a newer version doing things in parallel that ensures the method but not the order.

[JurajNyiri]

Thats a great idea, I would say instead of throwing the exception, we could convert it to a list instead. That way HA can then go into the list and find the correct property it is looking for.

I posted a comment on the PR.

[JurajNyiri]

I am going to accept PR and do the changes above, then I will push the code for you to test again.

[marcosngomezi]

not sure about the list, there is not going to be a way to know which one is first and second anyway, because can be in different order, and it would force tapo-control to always check if it is a dictionary or a list, are you sure that is worth making it a list? for what we have seen there are no devices that respond to both, if some day appears, there should be some extra research to understand why it responds to both, its an strange case to have two sirens on the same device

[JurajNyiri]

I have pushed a new commit making everything a list to keep it consistent and removed the exception.

https://github.com/JurajNyiri/pytapo/commit/a5f2213a7cd8bede335a0bff53ae792b937764db

Could you please test?

As for tapo integration I can refactor it so that it can accept these values as a list and look for the property it needs in the loop and as soon as it finds it break the loop.

[JurajNyiri]

Actually give me a second, I will try to make it more robust so that the number of requests always equals the number of responses, even if response fails...

[JurajNyiri]

@marcosngomezi pushed another commit. Let me know if it works please.

getConnectionType
[{'link_type': 'wifi', 'ssid': 'redacted', 'rssiValue': -44, 'rssi': '4'}]
getAlarmConfig
[False]
getAlarmPlan
[False]
getSirenTypeList
[False, False]
getSirenConfig
[False]
getLightTypeList
[False]
getSirenStatus
[False, False]

[JurajNyiri]

As for this PR with the new codebase of pytapo (assuming it will work).

What I think we can afford to do is inside getCamData(hass, controller): just change everything starting with data[ to data[...function...][0] except for getSirenTypeList and getSirenStatus. As we control pytapo code / version there is no need for universal loop or length checking on the rest.

As for the 2 that are there twice, both of these are not in there yet in the main branch, so for these what we can do is create a for loop and find the first expected value for them and save accordingly.

[marcosngomezi]

Its working:

Get most
Hub getConnectionType [{'link_type': 'ethernet'}]
Hub getAlarmConfig [False]
Hub getAlarmPlan [False]
Hub getSirenTypeList [{'siren_type_list': ['Doorbell Ring 1', 'Doorbell Ring 2', 'Doorbell Ring 3', 'Doorbell Ring 4', 'Doorbell Ring 5', 'Doorbell Ring 6', 'Doorbell Ring 7', 'Doorbell Ring 8', 'Doorbell Ring 9', 'Doorbell Ring 10', 'Phone Ring', 'Alarm 1', 'Alarm 2', 'Alarm 3', 'Alarm 4', 'Dripping Tap', 'Alarm 5', 'Connection 1', 'Connection 2']}, False]
Hub getSirenConfig [{'siren_type': 'Alarm 3', 'volume': '7', 'duration': 10}]
Hub getLightTypeList [False]
Hub getSirenStatus [{'status': 'off', 'time_left': 0}, False]
Camera getConnectionType [False]
Camera getAlarmConfig [{'enabled': 'off', 'alarm_mode': ['siren'], 'siren_type': 'Siren', 'light_type': 'flicker', 'siren_volume': 'high', 'siren_duration': 5}]
Camera getAlarmPlan [{'enabled': 'off', 'alarm_plan': '0000-0000,127'}]
Camera getSirenTypeList [{'siren_type_list': ['Siren', 'Tone', 'Red Alert']}, False]
Camera getSirenConfig [False]
Camera getLightTypeList [{'light_type_list': ['flicker']}]
Camera getSirenStatus [{'status': 'off'}, False]

[marcosngomezi]

Would you use same siren and config components for both types of sirens or keep it as siren and hub siren components in the integration? makes sense to have just one now I suppose

[JurajNyiri]

I think we can use the same, as the response is the same from the point of HA there is no difference, both have key "siren_type_list"

[JurajNyiri]

@marcosngomezi I have just pushed new version to pytapo https://pypi.org/manage/project/pytapo/release/3.3.20/

[JurajNyiri]

Let me know if you are OK with updating the rest like described above for breaking changes, if not I can do the changes on main. We should also now use the types reported by camera instead of hardcoding them inside the code.

[JurajNyiri]

Please make sure to test also if the integration uses the cached values, so that we do not re-request data that have been already retrieved via getMost. You should see a lot of "Found cached capability {check_function}, creating {cls.name}"

[marcosngomezi]

sure I can do the changes

[JurajNyiri]

Headsup, you will need to also modify this function since everything is now a list.

For example, in and "switch" in rawData["getLdc"]["image"] you would need rawData["getLdc"][0]["image"] I think.

[marcosngomezi]

Hi there @JurajNyiri not yet finished, numbers are remaining and some TODOs, but some questions and need of validation on other cameras rised.

[JurajNyiri]

Hey @marcosngomezi thank you for all your updates. Let me know if I can help in any way with this PR.

I will test it out once you say its ready and go through the code in detail.

[JurajNyiri]

Hi @marcosngomezi I had to push a new update to fix unrelated issue, please rebase this if you get the chance and see my comments above.

[marcosngomezi]

Hi there @JurajNyiri , sorry for the delay, trying to finish this PR, I think is ready for a review, also have tested it and is working as expected. There are two "issues" that I think are not related but maybe you know better:

• Config entry * has already been setup! Sound like for some reason is running twice the creation? or is it normal when restarting HA?
• Select and switch and select reports to be taking over 10 seconds, not sure if normal or there is something to improve.

[JurajNyiri]

Thank you!

Config entry * has already been setup!

I saw some users with this issue but have never been able to track it down or replicate it. Usually I told users to remove all entries in jsons for tapo_control and that solved it. Might be some misconfiguration, somehow, via some edge case...

Select and switch and select reports to be taking over 10 seconds

Some cameras are slower, some of mine take a rather long time to respond, not much we can do.

[JurajNyiri]

@marcosngomezi if you haven't yet, check my commit and that branch, just in case you missed my main comment on the review, might save you some time as I addressed most of the above, we just now need to make it work on both cameras as we discussed in the comments now.

[marcosngomezi]

Yes I saw it, lets resolve the pending comments and I will do the merge, remaining fixs and testing