-
-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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 Risco integration #36930
Add Risco integration #36930
Conversation
Ohhh i would love to see such an integration. |
Nice indeed, allthough official api is coming soon, really like this one |
Btw , is your based on riscocloud or are you retrieving based on the risco app? Risco app is better |
@pergolafabio this is based on the app API. As for official API coming soon, I wouldn't count on it, but if it ever gets released, I can update the underlying library to communicate through it, and hopefully leave the user facing parts unchanged. |
Cool , looking forward to it, and also the binary sensor ... I am now running the docker from mancioshell, his version is working great, also using the App API |
thanks, i will love this integration. it was just what I needed |
@pergolafabio working great is mostly a function of polling frequency (which I intend to make configurable. The mancioshell version is also configurable and defaults to polling every 5 seconds), and handling logouts. The advantages of using the app api are different (mainly that it's more future proof). |
Great! Gonna like it, and yes, good idea to make it a config option, I think I have mine setup to 3000 if if remember correctly, going lower was not good... |
Is it also possible to use multiple pin , or even use the pin coded from the existing users? |
I haven't implemented any pin code functionality yet, and haven't thought it through. The first implementation will most likely include a single front end pin code, which you can set to be the same as one of the control panel users. |
Is this staging for the next HA release? Or way too early? I have absolutely no idea :) |
@skank01 I wish I could tell you. This still needs to pass review, which might take a few back-and-forths, and I'll have to write the documentation for it. |
can we already test this ? or not possible since its config flow |
@pergolafabio the best way I know is to setup a development environment and checkout my branch. The developer docs can guide you, but keep in mind it's not trivial. |
Hmm, I don't have a dev env, guess I need to wait for release... Not possible to load as custom and yaml method? |
You can try as a custom component, but I haven't tested it, and don't know how it works. There's no YAML support, but maybe you can create a config entry in |
ok, gonna wait for, will test soon when it comes out |
The first version doesn't have a configurable interval just yet. This will be added in a future PR along with the |
no problem, take your time, i am already happy you are working on it,... then i can ditch mqtt and portainer for this, i have setup those 2 especially for risco from mancio but isnt it possible to make also yaml? so we can sideload your test/new features as a custom component, so we dont have to wait untill it get merged everytime |
Possible, yes :), but adds significantly more work for me, and given my limited time, I'd rather focus on tackling the next items in the roadmap (group arming, pin codes, data updater, binary sensors). |
Ok, gonna check then how I can load this as custom with that core file.... Gonna ask the community, I have no idea |
hmm, i cant find how to load as custom, seems not so easy when running HassOS |
Nice!!! Thank you! |
coordinator = hass.data[DOMAIN][config_entry.entry_id][DATA_COORDINATOR] | ||
entities = [ | ||
RiscoAlarm(hass, coordinator, partition_id) | ||
for partition_id in coordinator.data.partitions.keys() |
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.
Keys is the default iterator of a dict. We can remove .keys()
.
class RiscoAlarm(AlarmControlPanelEntity): | ||
"""Representation of a Risco partition.""" | ||
|
||
def __init__(self, hass, coordinator, partition_id): |
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.
Don't pass in hass
. It will be set on the entity when it has been added to home assistant.
|
||
def __init__(self, hass, coordinator, partition_id): | ||
"""Init the partition.""" | ||
self._hass = hass |
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.
Remove this.
if self._partition.disarmed: | ||
return STATE_ALARM_DISARMED | ||
|
||
return STATE_UNKNOWN |
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.
Use None
to represent unknown state.
|
||
async def async_alarm_arm_night(self, code=None): | ||
"""Send arm night command.""" | ||
await self._call_alarm_method("partial_arm") |
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.
We shouldn't have more than one service partially arm.
try: | ||
info = await validate_input(self.hass, user_input) | ||
|
||
return self.async_create_entry(title=info["title"], data=user_input) |
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.
Please move the entry creation to an else:
block.
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.
We should use the username as unique_id and guard from creating more than one entry with the same username.
new_callable=PropertyMock(return_value=partition_mocks), | ||
), patch( | ||
"homeassistant.components.risco.RiscoAPI.get_state", | ||
AsyncMock(return_value=alarm_mock), |
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.
Coroutines are patched with AsyncMock by default.
), patch( | ||
"homeassistant.components.risco.RiscoAPI.close", AsyncMock() | ||
): | ||
config_entry.add_to_hass(hass) |
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.
Move this out of the context manager.
Thanks @MartinHjelmare. I addressed those in #39143 BTW, how do I get the integration scored? |
In the PR, list the requirements bullets for the levels you want to pass and make a comment for each. |
@OnFreund is it possible to make sensors or get info from each detector? |
But this pr is about zones and I ask about movement detectors... |
Isn't a movement detector the same as a pir sensor? :) |
A zone is a collection of one or more detectors (not necessarily just movement, BTW) that work as an atomic unit. Risco doesn't really communicate in detectors, just zones. For example, when the alarm is triggered, you see the Zone that triggered it, not the individual detector. |
Yeah you are right but from time time I see in risco app one on my detectors that is actually trigerred by movement without trigerring alarm (zone is disarmed) changes its colour to red from green so they do communicate. |
That will be the binary sensor ;) |
What you're seeing in the app are zones, not individual detectors, just like in the integration (it even uses the same api). |
No no. I see individual detectors with menu to bypass and unbypass them and completely different thing is zones view with option to arm, disarm etc. |
Once again - what you're seeing in the app are zones, not detectors. (look at their names, they're even called "Zone 34", Zone 36aaaaaaaa"). |
"Yaron Office" and "Amir Office" in your screenshot are both partitions. |
Echh we don't understand eachother. They are called zone in this screen. In my house ther are called eg. kitchen, garage etc. and are grouped in partitions. In this screen partitions are called Yaron office etc. And as far as I understand You create one sensor for one partition right? This guy https://github.com/lucacalcaterra/risco-mqtt-bridge added ability to bypass and unbypass detectors. This is what I am talking about |
This isn't really the right place for this discussion, maybe we can move to discord? In any case, Yaron office, etc... are partitions. I don't know what "kitchen, garage, etc.." are, but bypassing in Risco works on Zones, not detectors (once again, Risco doesn't really have an externally visible concept of detectors, and the "Detectors" screen in the app shows Zones). |
OK. Now I think I get it. Detector = zone 👍 It doesn't matter. Look. I think that You made a great thing with that. Thank and looking forward to 0.115 to test it |
Still curios what "kitchen, garage, etc..." are in your setup. Do you have a screenshot with them? |
Kitchen, garage are just the names my installator gave them. By "them" I think about detectors just like "Detectors" screen is called in app but You call them zones. They have the same icon as eg. "21: PIR Detector" from above and are groupped under Partition name of my installation. In the scrren above it is "Yaron Office" and in my place it is eg. "Downstairs". |
The ideal world would be to use those detectors/zones to detect movement and eg. turn on/off the lights :) |
So yes, they are zones, and that's also the name Risco uses anywhere by the app (in the physical panel, for example). |
OK thanks |
Proposed change
A new integration for Risco alarm systems, through the Risco Cloud platform.
This is the first step, with a config flow and an alarm control panel for every partition.
Future PRs will add:
Fetching data throughDataUpdateCoordinator
, in preparation for:Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale: