Peer review my first SmartApp? detecting child eloping


(Martin Lariz) #1

Hello everyone, I have written my first SmartApp (other than a dumb one that just spit out some debug info for my own assistance).

The intent of this App is to detect if a child opens the front door without disabling the alert first. It is similar to a standard house alarm, but simplified in a way and always on. Pressing a defined button (multiple allowed since I have one inside and one outside) disables this alarm for a short time to allow ingress/egress without setting it off.

I submit it here for peer review as (1) this being my first App there are always possible gotchas I know nothing about, and (2) I always feel peer review is a good idea for code.

Link to github code.


(Kevin) #2

I didn’t notice any code related issues and it’s really fine as is, but since you asked for a review, here are a couple of things I did notice:

The “Alarm” capability only supports commands: off, siren, strobe, and both. The “Music Player” capability supports playTrack and stop, and since you don’t appear to be using any of the “Alarm” capability’s commands, it might make sense to change that input type to “capability.musicPlayer”.

If your siren doesn’t support that capability then that’s not really an option so as long as it’s working for you and you don’t plan on publishing it, I wouldn’t worry about it.

If you wanted to, you could use siren.hasCommand(“playTrack”) to check if the command is supported to prevent errors, but without those commands the SmartApp doesn’t really do anything so it really doesn’t matter.

Out of curiosity, what device and device handler are you using for your siren?

The only other thing I found a little odd is your use of 0 and 1 instead of true and false. The way you have it won’t cause any problems, but it makes it a little easier to read the code if you use booleans. They also allow you to use “if (state.alarming)” and “if (!state.alarming)” instead of having to use “if (state.alarming == 1)” and “if (state.alarming == 0)”

This doesn’t apply, but just so you know, if your preference input should be true/false, use the type “bool” instead of “boolean” because “boolean” is a string value and it makes it harder to work with.


(Tim Slagle) #3

Logic looks solid. I would have made the same suggestions as @krlaframboise


(Martin Lariz) #4

Thank you so much for the feedback, I truly appreciate it. I have made all the recommended changes.

The capability was just due to that being what I originally had set it to use and was going to use .alarm() but later changed to be able to have specific Track play.
As to the boolean represented as 0/1, mostly just old habits. That and I had originally over complicated the code as I didn’t have an interior button do disable. So I had some complex code that incremented the number such it would not alarm if <4, then the button would immediately add 4 but the switch closest to the door required an sequence to disable (up, up, down, down). However even with filtering turned off the response wasn’t consistent enough to rely on so I just delayed until a second button came.

With regards to your question as to what I am using for the siren, I am using an Aeon Labs Multifunction Doorbell, with a handler written by krlaframboise

While I don’t expect many people to need this App, I figured I would submit it anyways, since this use case wasn’t already covered. I know my case is not super common, but I’m sure there are others out there that may make use of it.

–mlariz

EDIT Hehe, read your name as “Kevin” when I read the response and didn’t notice until after writing the response here, that it was your device handler I was using. Allow me to extend my thanks for it here and now. :wink: