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 functionality to receive Events from Openhab and added methods to create and delete openhab items #22

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

galexey
Copy link

@galexey galexey commented Dec 13, 2020

I also created som testcases for the new functionality.

need write testcases still
basic tests for setting states and commands
added a method to delete items
finished documentation
@sim0nx sim0nx self-requested a review December 13, 2020 22:43
@sim0nx sim0nx self-assigned this Dec 13, 2020
@sim0nx
Copy link
Owner

sim0nx commented Dec 13, 2020

Hi @galexey
Thanks for your PR!
That is quite a big one to review though ;-)
Could you please make sure to rebase from master and cleanup your code according to pylint, flake8 and mypy as much as possible.

What are you thoughts on using aiohttp-sse instead of threading+sseclient?

renaming of literals to python style
# Conflicts:
#	openhab/client.py
#	openhab/items.py
cleaning code regarding new codestyles
@galexey
Copy link
Author

galexey commented Dec 14, 2020

Hi!
Jep, this one grew quite big, sorry.
I merged your changes and did some major cleanup (also applied python naming style)
I haven´t used aiohttp-sse yet, but it looks nice. I might get back to your proposal and have a deeper look into it later this month.

@galexey
Copy link
Author

galexey commented Dec 16, 2020

I need to rework Commandhandling a bit. expect another commit soon.

…ands.

changed handling of incoming values

wrote quite a lot of testcases for creation/deletion of items

wrote quite a lot of testcases for event handling
@@ -143,7 +375,7 @@ def fetch_all_items(self) -> typing.Dict[str, openhab.items.Item]:
Returns:
dict: Returns a dict with item names as key and item class instances as value.
"""
items = {} # type: dict
items = {} # data_type: dict
Copy link
Owner

Choose a reason for hiding this comment

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

this should really be type

openhab/items.py Outdated
class Item:
"""Base item class."""

types = [] # type: typing.List[typing.Type[openhab.types.CommandType]]
types = [] # data_type: typing.List[typing.Type[openhab.types.CommandType]]
Copy link
Owner

Choose a reason for hiding this comment

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

this should really be type

openhab/items.py Outdated
self._state = None # type: typing.Optional[typing.Any]
self._raw_state = None # type: typing.Optional[typing.Any] # raw state as returned by the server
self._members = {} # type: typing.Dict[str, typing.Any] # group members (key = item name), for none-group items it's empty
self._state = None # data_type: typing.Optional[typing.Any]
Copy link
Owner

Choose a reason for hiding this comment

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

this should really be type

openhab/items.py Outdated
self._raw_state = None # type: typing.Optional[typing.Any] # raw state as returned by the server
self._members = {} # type: typing.Dict[str, typing.Any] # group members (key = item name), for none-group items it's empty
self._state = None # data_type: typing.Optional[typing.Any]
self._raw_state = None # data_type: typing.Optional[typing.Any] # raw state as returned by the server
Copy link
Owner

Choose a reason for hiding this comment

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

this should really be type

openhab/items.py Outdated
self._members = {} # type: typing.Dict[str, typing.Any] # group members (key = item name), for none-group items it's empty
self._state = None # data_type: typing.Optional[typing.Any]
self._raw_state = None # data_type: typing.Optional[typing.Any] # raw state as returned by the server
self._raw_state_event = None # data_type: str # raw state as received from Serverevent
Copy link
Owner

Choose a reason for hiding this comment

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

this should really be type

openhab/items.py Outdated
self._state = None # data_type: typing.Optional[typing.Any]
self._raw_state = None # data_type: typing.Optional[typing.Any] # raw state as returned by the server
self._raw_state_event = None # data_type: str # raw state as received from Serverevent
self._members = {} # data_type: typing.Dict[str, typing.Any] # group members (key = item name), for none-group items it's empty
Copy link
Owner

Choose a reason for hiding this comment

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

this should really be type

openhab/items.py Outdated

def _rest_format(self, value: str) -> typing.Union[str, bytes]:
"""Format a value before submitting to openHAB."""
_value = value # type: typing.Union[str, bytes]
_value = value # data_type: typing.Union[str, bytes]
Copy link
Owner

Choose a reason for hiding this comment

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

this should really be type




# event = None
Copy link
Owner

Choose a reason for hiding this comment

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

no commented code blocks

else:
self.logger.debug("item '{}' not registered. ignoring the arrived event.".format(item_name))


Copy link
Owner

Choose a reason for hiding this comment

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

too many newlines

@sim0nx
Copy link
Owner

sim0nx commented Dec 28, 2020

A couple of change requests:

  • please do fix linting issues (too many newlines, no newline at end of file etc)
  • do not use data_type in docstrings. either data type or just leave type please
  • could you please strip the SSE stuff into a separate pull request so that we can differentiate between code reworks, new type support, more complete API support ... and the event stream handling ?
  • please also fix the tests which currently fail ... there are at least import errors

Thanks

…by Georges

some additions around event content
bugfixes
cleaned code to fix linting issues
@galexey
Copy link
Author

galexey commented Dec 30, 2020

Hi!
I cleaned the code as much as I cound and found reasonable.
the suggested aiohttp_sse_client works quite well and is a cleaner approach - thx for the hint.

However I can not split up the PRs to isolate the SSE-stuff, I had to change a lot of things in client, items and types modules for sse-implementation.

added voices
added voice interpreter

added cache for items
@sim0nx
Copy link
Owner

sim0nx commented Jan 3, 2021

Hi,
I cloned your changes into a local branch and spend some time cleaning things up a bit, namely docstrings and typing.

Could you please rebase your PR on that branch ( https://github.com/sim0nx/python-openhab/tree/galexey_pr ) ... if required open a new PR.
I think it is important to have cleanly typed code as much as possible.

I would also appreciate docstrings for methods which currently have none :-)...

Thanks

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.

None yet

2 participants