-
Notifications
You must be signed in to change notification settings - Fork 10
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
Support for partitionless panels e.g. iConnect2. #2
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @SimonThoustrup. Let's figure out a more object oriented approach. This will also ensure less risk of changes to one systems breaking the other.
Ideally, I would like the decision point to be as early as possible, say another RiscoAPI
implementation, and a single method that return either implementation depending on what type of system you have. From then on, the implementation should be separate with common base classes.
Thanks again!
@@ -36,41 +44,60 @@ | |||
|
|||
|
|||
class Partition: | |||
"""A representation of a Risco partition.""" | |||
"""A representation of a Risco partition (or panel in case of no partitions).""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be a different class in the no partition case, since basically all properties now have the same if self._partitions_available:
pattern. They can have a common base.
@@ -131,7 +161,10 @@ def __init__(self, raw): | |||
def partitions(self): | |||
"""Alarm partitions.""" | |||
if self._partitions is None: | |||
self._partitions = {p["id"]: Partition(p) for p in self._raw["partitions"]} | |||
if self._raw["partitions"] is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about this, but maybe better to return an empty collection and have a different property to return the partition-like object for non partition systems? Or maybe rename this property to panels
?
@@ -329,28 +363,50 @@ async def login(self, session=None): | |||
async def get_state(self): | |||
"""Get partitions and zones.""" | |||
resp = await self._site_post(STATE_URL, {}) | |||
if resp["state"]["status"]["partitions"] is None: | |||
self._partition_panel = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once again, we're repeating the same if self._partition_panel:
in multiple locations. Let's find an OO way to represent this better.
Thanks @OnFreund. I won’t have time to work on this for the next few weeks so if somebody else want’s to pick it up please feel free to do so. If not I’ll have a go when time permits. :-) |
@SimonThoustrup Hi Simon, are there any developments in this area since February? I am still struggling with my 'Partitionless' Risco (MyElas) iConnect2 alarmsystem. |
Hi. Sorry but I haven’t had time to do the refactoring. The patch I made still works though so you might get away with placing the patched risco.py in site-packages/pyrisco/ in your HA install. I hope I will have some time to fix this properly sometime next year.
Sent from my phone - likely short
Fra: Rookie-Ron ***@***.***>Sendt: torsdag, december 30, 2021 2:28 PMTil: OnFreund/pyriscoCc: SimonThoustrup; MentionEmne: Re: [OnFreund/pyrisco] Support for partitionless panels e.g. iConnect2. (#2)
@SimonThoustrup Hi Simon, are there any developments in this area since February? I am still struggling with my 'Partitionless' Risco (MyElas) iConnect2 alarmsystem.
Any workarounds that you are aware of?
—Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
@SimonThoustrup Thanks for the hint, no luck however. Maybe I do not have the process clear? This seems strange, as the file the error points to is your patched one, in which line 134 has your updated content. |
@SimonThoustrup @OnFreund
Traceback (most recent call last): Function "inschakelen voor vertrek" (=arm_away) does work as expected.
For this: Comments on the Log? Is there something I should do in the mapping of 'arm_home' versus 'partial_arm'? Is 'arm_home' supported anyway? |
Hi @Rookie-Ron How did you replace the 'risco.py' within the Home Assistant container? I cant find the path. I am running HA OS on oracle virtualbox. Can you please post a guide on how you did it? Thank you. |
I am running Home Assistant (core) in a Docker Container. The container has his own file structure, which you can access with command:
When you change to the main directory with 'risco.py' is under /usr/local/lib/python3.9/site-packages/pyrisco To replace the default file with the patched one, from the directory where you saved the patch, use:
One downside: after each update of Home Assistant you need to copy the patch to replace the original file. Good luck! |
Hey there, Are you having issues after update Homeassistant 2022.8.0? I replaced the default file with the patched one: and I get an error: Error while setting up risco platform for alarm_control_panel It worked flawlessly before the update. Any ideas? Thank you! |
The API has changed with the latest version, so whatever patch you're using will have to change accordingly. |
Thank you @OnFreund for the explanation. I am using @SimonThoustrup 's patch. Is this something that can be done easily? |
Don't know - you'll have to ask Simon... |
There hasn't been any real activity here for over 18 months, and the library has changed quite significantly in the mean time, so I'm closing this. |
Hi @SimonThoustrup are you planning to make changes to your patch? |
Hi @SimonThoustrup, same issue and question from my side as @ASOUAMES. Are you planning an update to your patch to match the new API? Any hints how to do it myself? |
Perhaps @SimonThoustrup is busy and cannot deal with this at the moment. Our only solution is for partitionless panels to be supported by @OnFreund. Hopefully one day. |
@ASOUAMES as I mentioned multiple times - I don't have a partition-less panel and I don't have a way to develop and test for it. |
Understood, what if I gave you access to my panel? My knowledge of python is s..t. |
|
lucacalcaterra/risco-mqtt-bridge#52 Found a work around. Perhaps it will help. |
Hey there.
I've added support for partitionless control panels such as the iConnect2 ref. home-assistant/core#40290. I've tested the changes in my Home Assistant and I have no errors in the log from the Risco integration. I hope I haven't broken anything for the panels that support partitions.
Please let me know if you think anything should be done differently.
Thanks.