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

Update decorators #1586

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 12 additions & 21 deletions miio/devicestatus.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,7 @@
import logging
import warnings
from enum import Enum
from typing import (
Callable,
Dict,
Optional,
Type,
Union,
get_args,
get_origin,
get_type_hints,
)
from typing import Dict, Optional, Type, Union, get_args, get_origin, get_type_hints

from .descriptors import (
BooleanSettingDescriptor,
Expand Down Expand Up @@ -131,7 +122,7 @@ def __getattr__(self, item):
return getattr(self._embedded[embed], prop)


def sensor(name: str, *, unit: str = "", **kwargs):
def sensor(name: str, *, unit: Optional[str] = None, **kwargs):
"""Syntactic sugar to create SensorDescriptor objects.

The information can be used by users of the library to programmatically find out what
Expand All @@ -143,7 +134,8 @@ def sensor(name: str, *, unit: str = "", **kwargs):
"""

def decorator_sensor(func):
property_name = func.__name__
property_name = str(func.__name__)
qualified_name = str(func.__qualname__)
rytilahti marked this conversation as resolved.
Show resolved Hide resolved

def _sensor_type_for_return_type(func):
rtype = get_type_hints(func).get("return")
Expand All @@ -157,8 +149,8 @@ def _sensor_type_for_return_type(func):

sensor_type = _sensor_type_for_return_type(func)
descriptor = SensorDescriptor(
id=str(property_name),
property=str(property_name),
id=qualified_name,
property=property_name,
name=name,
unit=unit,
type=sensor_type,
Expand All @@ -174,7 +166,6 @@ def _sensor_type_for_return_type(func):
def setting(
name: str,
*,
setter: Optional[Callable] = None,
setter_name: Optional[str] = None,
unit: Optional[str] = None,
min_value: Optional[int] = None,
Expand All @@ -196,17 +187,17 @@ def setting(
"""

def decorator_setting(func):
property_name = func.__name__
property_name = str(func.__name__)
qualified_name = str(func.__qualname__)

if setter is None and setter_name is None:
raise Exception("Either setter or setter_name needs to be defined")
if setter_name is None:
raise Exception("Setter_name needs to be defined")
Comment on lines +193 to +194
Copy link
Owner

@rytilahti rytilahti Nov 9, 2022

Choose a reason for hiding this comment

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

Why is setter parameter being removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since it will not work to put in a function directly in the decorator since the class object has not yet been created. Therefore always the setter_name schould be used and the binding of the setter methods schould take place when the class object gets created.

I tried to use the setter parameter in my code directly in the decorator, but I could not get it to work (I think it will never work).

Copy link
Owner

Choose a reason for hiding this comment

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

One way would be to give it a reference to a callable (like DeviceClass.set_something), and binding that to the instance object much like what is done for the setter_name one, no?`.

But that is not important at the moment, let's however keep the code in place just in case for the time being.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that works since how would you get the instance object from the DeviceClass.set_something callable. But I am quite new to all these decorators, still figuring it all out a bit.
Learning a lot from your coding skills!

Schould I just keep the setter parameter and add a Exception that it is not yet implemented instead?

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, just add an exception there. I'm also learning, so it's all good :-)

I think something like this could work for the binding, but I haven't tried it out:

from functools import partial
method = partial(setter, self)  # now the `self` is passed as the first parameter to the method calls

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made the exception in a seperate PR.


common_values = {
"id": str(property_name),
"property": str(property_name),
"id": qualified_name,
"property": property_name,
"name": name,
"unit": unit,
"setter": setter,
"setter_name": setter_name,
"extras": kwargs,
}
Expand Down