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

Update to V2 API with house address and alert cause #5

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

Conversation

0n3man
Copy link

@0n3man 0n3man commented Mar 2, 2021

I updated the code base to work with both noonlight V1 and V2 API. The V1 API works the same as before. With the V2 API you now add configuration values to report your house address and other items. In addition two new HA input_text fields were added so that the HA alerting automation can provide a reason for the alert and the desired service.

Copy link

@kit-klein kit-klein left a comment

Choose a reason for hiding this comment

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

Nice feature addition! Just a couple things to clean up.

README.md Outdated Show resolved Hide resolved
custom_components/noonlight/__init__.py Outdated Show resolved Hide resolved
custom_components/noonlight/switch.py Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
0n3man and others added 4 commits March 3, 2021 18:45
Co-authored-by: Kit Klein <[email protected]>
Co-authored-by: Kit Klein <[email protected]>
Removed commented out code
Changed to pull CONF_PIN from home assistant constants
Copy link

@kit-klein kit-klein left a comment

Choose a reason for hiding this comment

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

I did not build/test but the code changes LGTM.

Removed an extra common
@swahilitodd
Copy link

swahilitodd commented Mar 6, 2021

Stumbled across this while trying to see how best to send alarm reason to noonlight. Updated to this on my HASS install, but having trouble sending alarm reason. Can you update the instructions?

Sample Automation:
alias: Smoke Detected (Noonlight test)
description: ''
trigger:

  • platform: state
    entity_id: group.smokedetector_status
    to: 'on'
    condition: []
    action:
  • service: input_text.set_value
    data:
    value: fire
    target:
    entity_id: input_text.noonlight_service
  • service: switch.turn_on
    data: {}
    entity_id: switch.noonlight_switch
    mode: single

@0n3man
Copy link
Author

0n3man commented Mar 6, 2021

I suspect there is multiple ways to deal with the input_text, but this works for me

- alias: fire alarm on
  trigger:
  - platform: state
    entity_id: switch.testswitch
    to: 'on'
  action:
  - service: input_text.set_value
    data_template:
      entity_id: input_text.noonlight_service
      value: '{{ "fire" }}'
  - service: homeassistant.turn_on
    entity_id: switch.noonlight_switch
  id: 1bc421

@swahilitodd
Copy link

swahilitodd commented Mar 6, 2021 via email

@0n3man
Copy link
Author

0n3man commented Mar 6, 2021

Yes, noonlight has a developer portal. If you wanted to verify for yourself you can sign up for a developer API key at https://www.noonlight.com/developers. You then change the api_endpoint to https://api-sandbox.noonlight.com/dispatch/v1 and set your developers token vi:

dev_token: 'YOUR_DEVELOPER_TOKEN' 

Once you do this and restart HA you can use the noonlight debug link: https://developer.noonlight.com/app/debugger
to look at the message you've sent.

Note the developer token will not work with the production noonlight web api. I saw some people indicating you could just grab a developer key and not use the konnected.io code. I far as I can tell this incorrect as you still need to have some way to authenticate your key.

Alternatively to the above you can kickoff an alert on the production side and when noonlight contacts you tell them you're verifying your security system is working correctly and ask them to verify your address and service request. It would be good to have someone else run through the live test and respond back to the thread so that the code owner see more people than I have had success with this modification.

@swahilitodd
Copy link

swahilitodd commented Mar 6, 2021 via email

@swahilitodd
Copy link

the V2 update works as expected. A little clearer documentation may help, but was able to implement and verify via the Noonlight Developer portal and via the live monitoring. Happy to assist with clarifying instructions - thanks for the work on this update!

@heythisisnate
Copy link

@0n3man I just was able to test this today. Are you able to set internal defaults for input_text.alarm_cause and input_text.noonlight_service?
If these are not set in configuration.yaml, we get an unhandled exception when calling the service. It would be best if there was a valid default that didn't require any user configuration for the basic use case.

@0n3man
Copy link
Author

0n3man commented Mar 10, 2021

Yes, I'll go ahead and add. Updated in place to initial the alarm_cause and noonlight_service fields.

@0n3man
Copy link
Author

0n3man commented Apr 9, 2021

Nate is there something else required to get this code integrated?

@ShayaanF
Copy link

ShayaanF commented May 4, 2021

Is this getting merged? Because the integration as it is leaves a lot to be desired.

@ragaimeena
Copy link

Silly question
I have the original component installed, how do I upgrade to this one?
also the component needs a version number added before the June upgrade of the HASS

@Fabi0San
Copy link

I added @0n3man 's repository to HACS and installed it from there, I guess you will have to uninstall konnectected-io's version first. It worked wonderfully.

@heythisisnate Can we pretty please have this integrated? So many would benefit from it! Thanks!

@ragaimeena
Copy link

any chance to add a version number to the manifest?

@snicker
Copy link
Contributor

snicker commented Jun 3, 2021

Hey all,

Sorry it's taken a while to review this but I have a few issues with the way this is currently set up.

  1. This V2 API support should belong in the core library (https://github.com/konnected-io/noonlight-py) rather than quite a verbose payload coming from within the HA integration. This aligns with the general tenets of HA integrations as well.

  2. Requiring users to mandatorily create input_text entities is not very elegant- if a user forgets to do this, the integration will throw errors. Preference would be to make this part of a ConfigurationFlow with options for this integration if necessary.

    1. Planned improvements yet unexecuted would use device_class of triggered and monitored sensors to determine what service to call.
  3. If providing an address is optional, then there should be a check in the code to verify if the user provided address information, and if not, fall back to lat/long. I'll look into the rules on the V2 API, but I am curious if you can provide one or the other or both, and what is better. HA already has configuration parameters to set a lat and long, so it simplifies configuration by relying on that.

overall, pretty good, but I think this needs refinement before merging

@0n3man
Copy link
Author

0n3man commented Jun 3, 2021

This is very frustrating, structural comments 3 months after a working patch is submitted and 2 1/2 months after Nate provided suggestions. I implemented all of Nate's suggestions. Using an address instead of GPS coordinates while providing a means to specify the emergency service required makes this code base functional. Can't you merge the code as is? It is backward compatible with existing installation. It only requires adding address and input_text fields if you want the enhanced features. It makes the code compatible with noonlight V2 API. Assuming the patch isn't worth integrating can you give a timeline when you'll have these features implemented in the existing code base?

@snicker
Copy link
Contributor

snicker commented Jun 3, 2021

Hey @0n3man, I understand your frustration. You're welcome to continue running this code on your own installation and others can even do the same by cloning this pull request into their custom_components directory in their own HA install.

However, while this is functional, it is not the way the integration should work for all users for reasons I mentioned above. After further thought, I believe the best midpoint to address your goals will be to expose a new service within Home Assistant that allows specifying the specific alarm type. I will also integrate full V2 API coverage in the base library (I got caught up on the docs and changes last night) in the next couple weeks. Sorry for the thrash.

@heythisisnate
Copy link

I reviewed this with a member of the Noonlight dev team today. It's going to require some re-work unfortunately.

The main thing is that Noonlight does not want us to use the v2 API for this integration. The v2 API is intended more for 3rd-party integrations where you as the end-user would not have an account with Noonlight, but a 3rd-party service could trigger an alarm on your behalf.

In this use case, the v1 API is still most appropriate, because this is based off of a Noonlight account and subscription, and information (like your phone number and household members) linked to your Noonlight profile.

The v1 API should be able to support the desired enhancement of being able to specify an alarm type and street address. I'm working with @snicker to get this re-worked and done in a way that works for everyone. Thanks for your patience.

@0n3man
Copy link
Author

0n3man commented Jun 4, 2021 via email

@chrisaljoudi
Copy link

@heythisisnate What’s the overall status on this?

These are pretty simple APIs, so happy to help if there’s some dev work on the integration end needed… but would appreciate an update on the v1 and v2 semantics.

The documentation for Noonlight API v1 implies some service-side changes needed for providing “instructions”/context for an alarm:

827B9C76-843F-4A0E-9AEA-232BDA29EDCE

Any clarification to inspire a little confidence in the trajectory of API support here would be very helpful.

@0n3man
Copy link
Author

0n3man commented Jan 13, 2022

I understand this update probably will never be included. However if you're not going to accept this work it would be greatly appreciated if you added support for real location information (House addess) and secondary POC information. GPS does not work with row houses, town houses and apartments. Which means a large portion of the population can not use you're code to connect to noonlight. While at my house GPS would probably work, I still like having at least one additional contact on the alarm. I have a home cell number that anyone in the house can pick up on the multiple house phones and my personal cell phone. At my daughter's house in the city GPS is a non starter. I can't image the policy going around and knocking on a bunch of doors just to respond to an alarm company GPS based alert. Any update on when you might make these capabilities main stream? For now I guess I'll have to continue using my copy of the code.

@curt7000
Copy link

I suppose this pull is dead :( I really would like to include my Freeze and Water sensors into this service.

@swahilitodd
Copy link

Coming back to this after some time. The address works, but the Alarm cause doesn't update because the input_text entity/helper is read only. It can be changed using the Developer State tool, but not via service. Any idea on how to resolve?

@johnrobledo
Copy link

Hello, coming back in 2024. The V2 API no longer works

@johnrobledo
Copy link

@0n3man Hi Brian, the V2 integration doesnt work anymore and home assistant just says: line1 invalid for integration noonlight; line2 invalid for integration noonlight; and so on for all the additional tags. Can you help? 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.