-
Notifications
You must be signed in to change notification settings - Fork 3
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
Automatically close the garage when parked at home #18
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.
I like this idea and appreciate the PR. I've left a couple of implementation comments. Also, I think auto closing on park at home should be a configurable option, as not everyone may want this. Some people may park and then walk out to the street to get their mail, for example, and don't want the auto close, so it should be added to the config to enable this feature. It should probably be a per-car config item, as some drivers in a household may want this, and others may not.
I'm open to further discussion on this and the other comments I've left.
@@ -100,6 +101,10 @@ func CheckGeofence(config util.ConfigStruct, car *util.Car) { | |||
for i := 3; i > 0; i-- { | |||
if err := setGarageDoor(config, car.GarageDoor.MyQSerial, action); err == nil { | |||
// no error received, so breaking retry loop | |||
if action == "open" && len(responseChannel) == 0 { | |||
responseChannel <- true |
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'm not sure it makes sense to use a channel for this, as it's not tied to a specific car or garage, and if there's a message waiting to be consumed in the channel while a separate vehicle in the config parks elsewhere, it could falsely trigger the close process. I think it would make more sense to make this a bool
property of the car so that only the car that triggers the garage open can trigger a follow-up auto-close-on-park operation. So the value would be set to true
here, and then reset to false
when it's consumed (i.e. when the car shifts to Park).
@@ -113,6 +118,52 @@ func CheckGeofence(config util.ConfigStruct, car *util.Car) { | |||
car.GarageDoor.OpLock = false // release garage door's operation lock | |||
} | |||
|
|||
// when parked, check if the car is at the home location and close the garage door. | |||
func VailidateGeofenceAndClose(config util.ConfigStruct, car *util.Car, action string) { | |||
timeout := 40 |
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.
This timeout seems arbitrary, as any cooldown configured for more than a minute would likely never fire. This should probably either be configurable, or be dynamic based on the cooldown value. Another alternative is it could ignore oplock given that it's a special case operation.
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.
Switching to closing the garage from within the CheckGeofence function based on a car configuration should avoid the need for this timeout.
@@ -113,6 +118,52 @@ func CheckGeofence(config util.ConfigStruct, car *util.Car) { | |||
car.GarageDoor.OpLock = false // release garage door's operation lock | |||
} | |||
|
|||
// when parked, check if the car is at the home location and close the garage door. | |||
func VailidateGeofenceAndClose(config util.ConfigStruct, car *util.Car, action string) { |
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 don't know that we need the action string
parameter here, as the function's purpose (and name) is to close the garage, and this should never be called unless that is the intended operation. So specifying the action string as a parameter is redundant.
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.
Makes sense, Will be removed.
Thank you for taking the time to review the PR. I had originally planned on making this a configurable feature, but in testing found if I were using the garage more than once a day, it made it easy just to leave the feature on. I now realize there maybe some that don't need this feature. I will work on making this configurable per car. I can then use the configuration to detect if the garage should be auto closed from the CheckGeofence function and initiate the auto close from there, rather than passing the control back to main. |
@DjAnu Hey thanks for the love. I agree there are some other good solutions out there but none of them did what I wanted, so I started this project and open sourced it in hopes others might find it useful. If you have any issues getting it going, feel free to open an issue on the repo and I'm happy to help out, and it would help me learn what gaps there are in the documentation to try and make it as self-serve as possible. 😄 |
Hey thanks for your reply. I was able to set up telsamate using following instructions : I also updated the config file and added polygon coordinates. However as I am running docker on my synology NAS, I couldn't figure out how to load and run your image on my docker. Sorry I am not a programmer :-(. I am still searching to see if I can find any tutorial. If you have some time can you please give me some pointers. OR if we can hop on a char or something that would be great. Tx. |
Okay seems like I am getting close. I was able to load and run the docker using the instructions below: I am still running into 2 issues.
|
@DjAnu Congrats on getting that far.
Hope that helps. |
@DjAnu Based on that log it looks like the host should be So instead of this:
You'd have this:
Note I removed the Hope that helps. |
Yes, I had figured that out and had made changes accordingly. Do you use
any channel / app where we can chat a little bit if you don't mind. It
could be reditt or google voice or anything you use. I think I am very
close just can't figure out MQTT connection. Truly appreciate your help.
…On Sat, Sep 16, 2023 at 8:21 PM brchri ***@***.***> wrote:
@DjAnu <https://github.com/DjAnu> Based on that log it looks like the
host should be 192.168.0.11. I took a look at the guide you linked, and
it looks like the docker-compose.yml example
<https://geekjournal.ch/wp-content/uploads/2022/01/docker-compose-example.txt>
they provided has commented out the port exposure for the MQTT broker. If
you also have those commented out in your config file, you'll want to
remove the comments so the ports will actually be exposed. That would cause
your problem.
So instead of this:
mosquitto:
image: eclipse-mosquitto:2
restart: always
command: mosquitto -c /mosquitto-no-auth.conf
# ports:
# - 1883:1883
volumes:
- mosquitto-conf:/mosquitto/config
- mosquitto-data:/mosquitto/data
You'd have this:
mosquitto:
image: eclipse-mosquitto:2
restart: always
command: mosquitto -c /mosquitto-no-auth.conf
ports:
- 1883:1883
volumes:
- mosquitto-conf:/mosquitto/config
- mosquitto-data:/mosquitto/data
Note I removed the # from two lines.
Hope that helps.
—
Reply to this email directly, view it on GitHub
<#18 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABJLDEXPPFYMWAFWNORNIIDX2ZUFPANCNFSM6AAAAAA4RATAIU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
No description provided.