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

Added a decorator to connect to the heat pump #131

Merged
merged 2 commits into from
Feb 5, 2024

Conversation

Guzz-T
Copy link
Contributor

@Guzz-T Guzz-T commented Jul 21, 2023

Using a decorator we can simply re-use the code to connect to the heat pump. This allows us to offers several read / write functions like only read the paramters.

Fixes #130

@gerw
Copy link
Collaborator

gerw commented Jul 22, 2023

Shouldn't it be possible to use your _connect as a "real" decorator (with @) around the read/write methods?

@Guzz-T
Copy link
Contributor Author

Guzz-T commented Jul 22, 2023

Unfortunately, a "real" decorator cannot use self as an argument. Without self, however, the internal fields cannot be accessed correctly. Also pylint complains.

@gerw
Copy link
Collaborator

gerw commented Jul 22, 2023

I tried a little bit and it seems to be possible with a "real" decorator:

def decorator(func):
    def _decorator(self, *args, **kwargs):
        self.num_calls += 1
        return func(self, *args, **kwargs)
    return _decorator

class TestSample:
    def __init__(self):
        self.num_calls = 0

    @decorator
    def dummy(self, foo):
        print(foo)

t = TestSample()

print(f"t.num_calls: {t.num_calls}")
t.dummy(2)
print(f"t.num_calls: {t.num_calls}")

There are also no complaints by pylint (only concerning missing docstrings and so on).

@Guzz-T
Copy link
Contributor Author

Guzz-T commented Jul 22, 2023

Have you tried accessing private fields like self._num_calls instead of self.num_calls?

I previously had the decorator inside the class. If you place it outside - like in your example - you at least bypass the fact that pylint requires a self argument.

@gerw
Copy link
Collaborator

gerw commented Jul 22, 2023

Yes, the following is pylint-compatible:

"""Random module"""
def decorator(func):
    """Random decorator"""
    def _decorator(self, *args, **kwargs):
        self._num_calls += 1
        return func(self, *args, **kwargs)
    return _decorator

class TestSample:
    """Random class"""
    def __init__(self):
        self._num_calls = 0

    def another_method(self):
        """Random member fctn"""
        return self._num_calls

    @decorator
    def dummy(self, foobar):
        """Random member fctn"""
        print(foobar)

t = TestSample()

print(t.another_method())
t.dummy(2)
print(t.another_method())

luxtronik/__init__.py Outdated Show resolved Hide resolved
luxtronik/__init__.py Outdated Show resolved Hide resolved
luxtronik/__init__.py Outdated Show resolved Hide resolved
luxtronik/__init__.py Outdated Show resolved Hide resolved
luxtronik/__init__.py Outdated Show resolved Hide resolved
luxtronik/__init__.py Outdated Show resolved Hide resolved
luxtronik/__init__.py Outdated Show resolved Hide resolved
@kbabioch
Copy link
Collaborator

@Guzz-T Could you elaborate on what the vision here is? How do you envision to use this in the future?

@github-actions
Copy link

github-actions bot commented Jul 23, 2023

Coverage

Coverage Report
FileStmtsMissCoverMissing
luxtronik
   __init__.py13410621%38–54, 61–66, 69–73, 79, 83, 102–113, 116–119, 122–142, 145–160, 163–180, 183–198, 202–204, 208–209, 213–214
   __main__.py21210%2–48
   datatypes.py2351295%37, 42, 47, 57, 72–75, 80–83, 92
   discover.py433421%25–77
luxtronik/scripts
   dump_changes.py44440%5–93
   dump_luxtronik.py28280%5–64
TOTAL58824558% 

Tests Skipped Failures Errors Time
110 4 💤 0 ❌ 0 🔥 0.637s ⏱️

@Guzz-T
Copy link
Contributor Author

Guzz-T commented Jul 23, 2023

@Guzz-T Could you elaborate on what the vision here is? How do you envision to use this in the future?

@kbabioch: Does the comment in #130 answer your question?

@Guzz-T
Copy link
Contributor Author

Guzz-T commented Jul 23, 2023

Yes, the following is pylint-compatible:

@gerw: I followed your suggestions, but now we get several W0212 pylint warnings

@gerw
Copy link
Collaborator

gerw commented Jul 23, 2023

This is strange. Pylist does not complain about something like self._num_calls += 1, but about self._num_calls = self._num_calls + 1.

@Guzz-T
Copy link
Contributor Author

Guzz-T commented Jul 26, 2023

This is strange. Pylist does not complain about something like self._num_calls += 1, but about self._num_calls = self._num_calls + 1.

I reverted the changes because it would just be a different style of the decorator

@gerw
Copy link
Collaborator

gerw commented Jul 27, 2023

Since we are not saving the decorated function, it seems not necessary to implement this as a decorator. What about something like

    def _with_lock_and_connect(self, func, *args, **kwargs):
        with self._lock:
            is_none = self._socket is None
            if is_none:
                self._socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
            if is_none or is_socket_closed(self._socket):
                self._socket.connect((self._host, self._port))
                LOGGER.info(
                    "Connected to Luxtronik heat pump %s:%s", self._host, self._port
                )
            return func(*args, **kwargs)


    def read(self):
        return self._with_lock_and_connect(self._read)

?

@Guzz-T
Copy link
Contributor Author

Guzz-T commented Jul 31, 2023

There isn't just one way of doing it. My version works like a "@"-decorator.

@Guzz-T Guzz-T force-pushed the issue/130-decorator branch 2 times, most recently from bd14ce5 to 2cbd44a Compare January 9, 2024 23:12
@Guzz-T
Copy link
Contributor Author

Guzz-T commented Jan 10, 2024

Is there still interest in these changes?

luxtronik/__init__.py Outdated Show resolved Hide resolved
@Bouni Bouni merged commit 3bfa473 into Bouni:main Feb 5, 2024
6 checks passed
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.

Get rid of _read_after_write()
4 participants