search

guerrerotook / securitas-direct-new-api

This repository contains the new securitas direct API that can be integrated in Home Assistant
Apache License 2.0
76 stars 30 forks source link

enabling no-code setup and fixing codes starting w/0 #181

Closed cantupaz closed 8 months ago

cantupaz commented 8 months ago

BTW. this PR probably fixes https://github.com/guerrerotook/securitas-direct-new-api/issues/153 and https://github.com/guerrerotook/securitas-direct-new-api/issues/149 (if I understood their problem) and some random errors I was seeing where the code is trying to access a property of a string... https://github.com/guerrerotook/securitas-direct-new-api/issues/144

cantupaz commented 8 months ago

I merged all the changes that separate the api from the HA integration (https://github.com/guerrerotook/securitas-direct-new-api/issues/184). It's been working for a couple of days with no errors and fixes the API changes that Securitas introduced in the new year.

What needs some testing is that I made a couple of changes to the actual API calls because I couldn't arm or disarm my alarm completely. Instead of DARM1, this version is sending DARM1DARMPERI1. And instead of ARM1, I'm sending ARM1PERI1. If that causes problems, I have an idea on how to fix them.

guerrerotook commented 8 months ago

Let me take a look at it @cantupaz

guerrerotook commented 8 months ago

@cantupaz I will let you that you merge it when you like it, from there I will create a new release!

cantupaz commented 8 months ago

I merged. Thanks for the access. I'm happy to help with this as long as I have a Securitas alarm.

Could you test air quality sensors? I don't have those in my installation and I have no way of testing.

It would also be good to test the arming and disarming, since I changed the codes sent to Securitas

guerrerotook commented 8 months ago

I merged. Thanks for the access. I'm happy to help with this as long as I have a Securitas alarm.

Could you test air quality sensors? I don't have those in my installation and I have no way of testing.

It would also be good to test the arming and disarming, since I changed the codes sent to Securitas

I'm going to test this on my installation to see how it goes and then I will release a new version. Happy to communicate with you using telegram or WhatsApp if you prefer it.

lukelalo commented 8 months ago

Sorry, but is this piece of code right?

image

Shouldn't the value be installation instead of instalation?

image

Regards! Pablo García

guerrerotook commented 8 months ago

Hi @lukelalo what do you mean by that? Can you elaborate? That was a rename of a typo I think.

lukelalo commented 8 months ago

Yes, the CONF_INSTALLATION_KEY was renamed, but the "instalation" value assigned wasn't (line 51 on customcomponents/securitas/ init _.py).

I mean that if the code is using devices.installation, shouldn't the CONF_INSTALLATION_KEY value be "installation" with double L instead of "instalation"?

Regards Pablo García

cantupaz commented 8 months ago

I agree that the value of the CONF_INSTALLATION_KEY should be installation, but I decided against changing it because it would break the existing configs of users that are using this integration. Maybe something to change later. Is something not working as you'd expect?

lukelalo commented 8 months ago

No, I was just pointing that the "instalation" value could crash with the installation attribute in devices. If you are only using this value to retrieve the config, you're right to keep the configuration value to avoid regression on users.

Regards Pablo García.