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

Add force white lamp state #31

Merged
merged 3 commits into from
May 6, 2022

Conversation

TassSinclair
Copy link
Contributor

@TassSinclair TassSinclair commented May 4, 2022

Context

The Tapo C320WS supports a floodlight, referred to in the API as a "Whitelamp". This PR adds support for turning this on and off.

The PR also includes a refactor to simplify how "common' and "switch" values are set/get, and adds some abstractions so library consumers don't need to unpack response data.

Additions

  • Add getForceWhitelampState and setForceWhitelampState for getting and setting floodlight status
  • Sync private __getImageSwitch / __setImageSwitch and __getImageCommon / __setImageCommon functionality
  • Add getLightFrequencyMode and getDayNightMode to avoid depending on API implementation details
  • Revise setDayNightMode to be consistent with setLightFrequencyMode

Caveats

Some of the existing tests don't work with the C320WS, for example, due to the lack of a motor. Logging the failures here for future investigation, we might want to keep track of individual camera model capabilities, or simply bail on tests that return a -40106.

FAILED test_pytapo.py::test_getMotionDetection - AssertionError: assert 'enhanced' in {'.name': 'motion_det', '.type': 'on_off', 'digital_sensitivity': '50', 'enabled': 'on', ...}
FAILED test_pytapo.py::test_getAutoTrackTarget - Exception: Error: Parameter to get/do does not exist Response:{"target_track": {}, "error_code": -40106}
FAILED test_pytapo.py::test_getMotorCapability - Exception: Error: Parameter to get/do does not exist Response:{"motor": {}, "error_code": -40106}
FAILED test_pytapo.py::test_moveMotor - Exception: Error: Parameter to get/do does not exist Response:{"error_code": -40106}
FAILED test_pytapo.py::test_moveMotorStep - Exception: Error: Parameter to get/do does not exist Response:{"error_code": -40106}
FAILED test_pytapo.py::test_setAutoTrackTarget - Exception: Error: Parameter to get/do does not exist Response:{"target_track": {}, "error_code": -40106}
FAILED test_pytapo.py::test_errorMessage - assert 'Privacy mode is ON, not able to execute' in 'Error: Parameter to get/do does not exist Response:{"error_code": -40106}'
FAILED test_pytapo.py::test_preset - Exception: Error: Parameter to get/do does not exist Response:{"error_code": -40106}
FAILED test_pytapo.py::test_getRecordings - KeyError: 'playback'
FAILED test_pytapo.py::test_calibrateMotor - Exception: Error: Parameter to get/do does not exist Response:{"error_code": -40106}

Test sample

...
tapo.setForceWhitelampState(True)
time.sleep(1)
tapo.setForceWhitelampState(False)
Untitled.mov

return self.performRequest(
{"method": "set", "image": {"common": {"inf_type": inf_type}}}
)

Copy link
Contributor Author

@TassSinclair TassSinclair May 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this further down the file to be near setLightFrequencyMode.

@@ -355,6 +349,7 @@ def getRecordings(self, date):
)["result"]["responses"][0]["result"]["playback"]["search_video_results"]

def getCommonImage(self):
warn("Prefer to use a specific value getter", DeprecationWarning, stacklevel=2)
return self.performRequest({"method": "get", "image": {"name": "common"}})
Copy link
Contributor Author

@TassSinclair TassSinclair May 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method forces the caller to know about the response structure, we should move towards specific getters for a clearer abstraction.

Copy link
Owner

@JurajNyiri JurajNyiri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First thank you for such a detailed PR and your contribution! I really like the code changes and the new feature.

I tested this on my C200 and found 2 changes we need to make;

def __getImageSwitch(self, switch: str) -> str:
        data = self.performRequest({"method": "get", "image": {"name": ["switch"]}})
>       return data["image"]["switch"][switch]
E       KeyError: 'force_wtl_state'

pytapo/__init__.py:424: KeyError

We need to handle this properly - if the key is not in the function, we need to raise an Exception.

pytapo/__init__.py Outdated Show resolved Hide resolved
pytapo/__init__.py Outdated Show resolved Hide resolved
@JurajNyiri
Copy link
Owner

As for the unit tests I really like the idea, I have created #32 .

@TassSinclair
Copy link
Contributor Author

Thanks @JurajNyiri , I have raised exceptions for when the switches or fields are not supported by the camera, and did the same for the playback (which is not supported by the C329WS).

Feel free to modify the exception messages if another phrasing makes more sense to you!

Copy link
Owner

@JurajNyiri JurajNyiri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 Thank you! Going to merge & deploy this as a new version to pypi.

@JurajNyiri JurajNyiri merged commit f5cc902 into JurajNyiri:main May 6, 2022
@TassSinclair TassSinclair deleted the add-force-white-lamp-state branch May 6, 2022 12:01
@JurajNyiri
Copy link
Owner

JurajNyiri commented May 6, 2022

Deployed to pypi as 2.2.

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.

2 participants