-
Notifications
You must be signed in to change notification settings - Fork 43
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
Create Insteon scenes in the config file. #25
Comments
Scene creation can currently be done via the command line tool (or MQTT) using the db-add command on devices or the modem virtual scenes. This enhancement should be specifically for creating a syntax in the config file for making this easier to do and to create a "sync" style command that pushes the scenes out to the devices. This also makes it easier to replace devices and serves as a back up for the insteon scenes. |
My first though is that the modem scene should be automatically created (the only downside to doing that is there are only 240'ish scenes available) and used by name. There should really be no need to manage the modem scene ID numbers by hand. Here's a couple of possible input formats. A longer, more yaml style one and a shorter, misterhouse style one. Default group is 1, level is 100%.
|
I would tend to agree about using by name, and not having the user worry about scene numbers at all and always including the PLM, just a note that there are a maximum of 240 scenes in the config file would be sufficient I think. Really, if you're doing real home automation any actual "scenes" are likely to be handled by your automation software and these are really just for creating n-way switches. If you need more than 240 n-way switches in your mansion you probably should get a commercial lighting system anyway :) Even though it's more verbose I think the yaml style is much more readable for novice users especially so I'd tend to lean that direction. The only note about using names instead of numbers I might have is that when running debug commands such as printing the link database it would be nice if the device/scene name showed in addition to the address/number e.g. in parentheses. This would save quite a bit of back and forth looking at the config file. |
I vote for yaml (home-assistant) style but either one works! BTW How's development going with this? I'm assuming this must be a complicated part. |
Sorry - it probably won't get done any time soon. I've got a ton of house projects that have to get done and don't have a lot of free time right now.. |
BTW, someone linked this on another git project, I thought it might be handy for you if you haven't seen it before Insteon Commands. |
So I am rather eager to get something like this started. I have more than 75 devices with multiple n-way switches and a number of custom scenes. This feature is one of the things that prevents me from moving from my misterhouse setup to home-assistant. @TD22057 the "shorter, misterhouse style one" seems rather awful from a user experience. I would vote for the more verbose design. I don't know how I feel about always creating a PLM scene for any link. It is different than any other Insteon design I have worked on before (that may be a good thing). Pros:
Cons:
I think I am willing to go along with the modem being automatically included in the scene. But we should be aware that people may run into issues in the future. |
I would like to see a way to use the config.yaml file to define the scene, and have the PLM go out and build the links to create the scene, with itself one of the responders, so it can always know the state of things. Why? well the 240 PLM scene limit may be a thing, but in the unlikely event HA is down, it would be nice to press a KPL button or pass a motion sensor, and have the right thing happen all the time. And even when all is well, and in fact, when all is perfect, a PLM scene has to be triggered by an event, intepret the trigger, and then activate the scene, which take a bit of time. At best, it's about a second, but that's a really long time when you walk into a room and expect the lights to come on (for instance). |
FYI - I'm in the middle of rewiring my house so it's going to be a little while before I can really work on this. Here is where I'm at with an initial design idea if anyone wants to start coding.
|
My coding isn't as ingenious or succinct as your's, but I can take a stab at it. @TD22057 A few things (sorry ahead of time for the wall of words):
I would get the following link pairs created:
Bathroom_sw1&2 will already have The only benefit I can see from this is that if I want the Bathroom switches to appear as a single object in HomeAssistant, I can use the Bathroom PLM scene as switch rather than doing some coding to merge Bathroom_sw1&2.
|
Some quick answers: The main idea w/ this design to make easily understood components. yaml <-> db is fairly straightforward. Then db vs db comparison is also reasonably easy. Those can both be coded and tested independently. Once they work, it will be easy to combine them to get scene syncing. |
FYI: I added a new branch scene_sync to start working on this. I added the method db.Device.diff() to find the diffs between two device db's and return the result as a db.DbDiff object. Also added Device.apply_diff() to update a device db from the diff. Still need to test it out and then do the same for the modem. This is to start implementing item 2 in my notes above. |
Awesome. I keep meaning to work on this, but fail to have the time. Hopefully things will slow down soon. |
Here is my scene syntax proposal.
|
Another question: should the scene file initiate pairing for a new device? I'm assuming the scene system will start by creating all the links that are required for the device to operate (basically like a pair() command). Then add in all the links that are in the scenes description file. Finally update the links on the devices to match the results. So the question is should the system try and initiate by-directional pairing between the modem and the device first if needed? That might need to be a future enhancement - I'm using the pair command manually now and it works great but I'm not sure how automatable it is. |
SIs think the syntax as you have it demonstrated here is great! I’m wondering about adding or removing scene responders- if I just modify the confit.yaml, I’d like to know that on reload, it will just be taken care of.
However, I think that automating the pairing function should be out is scope for this issue, and in general, I think it should be a manual process.
…-Dave
On Dec 7, 2018, at 1:04 AM, Ted Drain ***@***.***> wrote:
Another question: should the scene file initiate pairing for a new device? I'm assuming the scene system will start by creating all the links that are required for the device to operate (basically like a pair() command). Then add in all the links that are in the scenes description file. Finally update the links on the devices to match the results. So the question is should the system try and initiate by-directional pairing between the modem and the device first if needed? That might need to be a future enhancement - I'm using the pair command manually now and it works great but I'm not sure how automatable it is.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
This is the trade-off that happens constantly with Insteon. There are a lot of steps and requirements and it is tempting to try and automate all of that to make it easier for users. The problem is, the failure rate for the insteon protocol is high, and the decision tree of what to do in each failure case gets large particularly when there are multiple steps that have to be taken. Generally the solution has been to have the incremental steps as separate operations, and then to work on adding batch operations later. But always leaving the separate operations to allow users to clean up from the inevitable failed batch operations. |
Gents,
I just saw my response from last night. I was coming out of anesthesia. It appears my wife was right to not give me my work laptop!
🙈but I’m feeling much better now. 🤪
As Kevin said, it’s hard to know when to say when. It seems that particularly with Insteon, trying to create “the one to rule them all” is really tilting at a windmill, and the returns start to diminish amazingly quickly.
Personally I’d much rather have the current process for pairing devices, as it is, and then this scene creation syntax.
At that point, I can take a new Insteon device, install it where I want it, and pair it to the plm. I then can add it to any scene I want, whenever I want.
If I were to create a scene containing a device which hadn’t been paired to the plm, I’d be quite satisfied with a warning message in the log to the effect of “scene <scene name in config file> contains unknown controller/responder <problem device name>”
Dave
… On Dec 7, 2018, at 11:28 AM, Kevin Robert Keegan ***@***.***> wrote:
This is the trade-off that happens constantly with Insteon. There are a lot of steps and requirements and it is tempting to try and automate all of that to make it easier for users. The problem is, the failure rate for the insteon protocol is high, and the decision tree of what to do in each failure case gets large particularly when there are multiple steps that have to be taken.
Generally the solution has been to have the incremental steps as separate operations, and then to work on adding batch operations later. But always leaving the separate operations to allow users to clean up from the inevitable failed batch operations.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
OK - thanks for the comments everyone. I tend to agree with you that linking (modem<->device group 1) and pairing (modem<-> device other groups) should be done first and the sene manager should handle just the user defined insteon scenes. I think the only thing this does to the code is that I'll my database differencing idea doesn't work because I won't know what the modem/device links should be for the pairing. I'm not sure if it's more work to make something that understands that or something that can correctly ignore the modem/device links to control things while still creating and syncing the correct modem device links that were requested. I'll start prototyping it and see what works out better. |
How is this going? I see the scene_sync branch contains a start of a difference calculator and the syncing process. From what you have my assumptions is that a device would have a self.db_config attribute that is the config database? And then we would run self.db_config.diff(self.db)? Btw, what does rhs stand for? Just curious. It seems like we need:
I don't want to duplicate work, so what can I help with? It seems like the Item 1 yaml->db code is self contained by a fair amount of work, I can tackle that if you like. |
rhs = right hand side. I tend to think in equation land: lhs + rhs = result (left hand side + right hand side) so I think I default to that when I'm not sure what to call the input. Sounds like a plan. I was struggling with the best way to handle links created by the pair command (your item 3). The pair command currently contains the info about what links need to be there with the modem but in some cases (IOLinc) there is an extra link that is defined at run time. I don't want to duplicate that in the syncing system. My current best idea is to restructure all the pair() commands to have each class have a method to build the set of needed links and return it. Then pair() would be moved to the base class and it would call that method and then loop over those and add any that are missing. This way the db sync system could call the same method on the devices to get the needed links to/from the modem to combine with the links the user is requesting. IOLinc is a special case because it needs a scene command to work properly. That would probably need to be added to the new meta data system in the device so we can remember the scene ID being used for that. That way IOLinc can return that as part of it's required links. And if it doesn't exist, we probably need a "unspecified" group number option to say we need a link but that the exact number doesn't matter. |
ps: I haven't had much time to devote to this so feel free to work on any part of it and send PR's to the sync branch. |
I have started working on this. FYI both the Yaml->db and the db->Yaml functions are inside the Modem class. I know you had originally envision them as being static functions inside the db classes. I wasn't able to do this. If the function is in the db class, each db has to load the entire config and process the whole thing to determine which scenes apply to that db. As a result the same links get processed over and over again. At the modem level, we can read the config once and dispatch the scene links to each db as we encounter them. |
I am having a hard time handling the lack of a modem group when the modem is a controller. I agree from a user experience perspective, having them not define a group is the best option. But I think once we add the links to the devices, we need to go back and edit the config file to have the proper group number. That way when the user reruns sync_links, we don't accidentally delete and recreate a whole bunch of links. |
Ugh, no writing to the config file likely won't work. That would destroy all comments and ordering in the config file. It looks like maybe we could switch to ruamel.yaml which might solve the issue. Alternatively, maybe we can make scene names mandatory. For modem controlled scenes they basically have to be if the user doesn't specify a group, otherwise how else does the user call the correct scene? We can then save the scene name in the modem db file. But this gets messy if the user modifies a scene name. I would really like to avoid trying to match a scene defined in a config file with one in the modem database using the list of responders. I just worry that many scenes may contain overlapping device members. Imagine the following scenes
Now imagine that the link to Dev1 for House Scene on the modem gets deleted somehow. How do we decide which group number House Scene should be? I think the correct answer is to say the one that requires the least amount of changes to make things how they should be. But that decision requires inspecting the entire link pair, including looking at each responder device to see if it has links back to the modem. Ugh, I guess that is the correct answer, it just sounds real messy. |
Grrr, sorting by changes that need to be made doesn't work well if our cached database is out of date either. Imagine someone wipes out their data directory. We could force a refresh in running the routine but that adds complexity particularly if we want to avoid unnecessary traffic by only refreshing the devices we think need changes. But that process looks silly, 1) compare state decide on course of action requiring least changes, 2) check if refresh is needed on any device slated for changes, 3) re-evaluate 1 after if any refresh occurs. tldr; I am clearly wandering and could use some input. Summing up my ideas (sorry this is still long):
We can certainly do this on startup and may be able to wait to do it until a user calls sync_links in a way that requires making a decision.
Cons:
Cons:
Cons:
If ruamel.yaml works, I am leaning towards 1 |
😀 50 commits and 2k lines of code. But it works! Scene syncing and importing is nearly done. I need to write the unit tests, some documentation, and clean up some code. But I can probably be done by Christmas. I tried it out on my entire network tonight. I was able to import 85 devices with over 200 scenes and it all looks good. After the scenes are imported, you still have to do some merging, organizing, and cleaning up, but all doable. So exciting! |
This is very exciting news. I had kind of forgotten about this project. I just ordered a serial interface and I have time next week to test. In the last several months I finally started learning python. Most of my work over the last 10+ years has been in Objective C and JavaScript. Im still a python newb but I am not new to development. Let me know if there is anything I can do to help. |
@krkeegan That is the best news I've had in weeks! I'm looking forward to seeing what you've done. |
Anyone can try it out. My branch is: https://github.com/krkeegan/insteon-mqtt/tree/scene_sync It is current with the dev branch of this repo. |
I wrote the unit tests tonight and caught a few small bugs in the process. I will spend some time hopefully tomorrow completing the documentation and then a can make a pull request. |
It was quite a project getting insteon_mqtt configured and running join/pair/refresh on all of my devices but I finally finished. As I test do you want bugs/errors reported here or in your repo? Running import-scenes is always timing out for me. I have increased the timeout to 60 and I have tried running it on devices that only have many links and devices with only a few links. Also I am running this on my 2018 MacBook Pro right now so performance shouldn't be the problem. |
Awesome! I am optimistic. So report errors in this issue thread for now. If they are stubborn issues, we can make them issues in this repo. I have hopes that we can merge this branch soon. As to your timing out issue, this was a problem for a bit, but I pushed a new commit a few days ago that should fix the issue. Can you try fetching and running again? |
Great. Also keep in mind that until #31 is done, this probably won't work for battery powered devices. |
I am running with the commit from 12/27. I don't have any battery powered devices in operation right now or in the config. There may be some old links floating around though. |
Interesting, well we may have a real bug on our hands.
|
Ok it appears to only be a command line problem. I just ran it from MQTT and it worked.
I just ran |
Hmm, alright. Well glad some things are working. So it seems to be command line related. What happens if you run:
|
Same thing. I get a timeout
This is the db when I call
|
Ok this doesn't appear to be related to your branch at all. With the release version I get the same thing. In fact none of the command line seems to work except for start. |
@respectTheCode Check your mqtt server and make sure there aren't protections on topics. The only way that should occur is if the command line client can't subscribe to the session topic to receive the messages back (since from the logs, the messages are being sent long before the 10 sec time out would occur). Looking at the code in cmd_line/util.py, it could be written to be a lot more robust. It doesn't handle errors from the connection or subscription very well and doesn't wait for things like a CONACK before running. So it might be possible if the MQTT server is slow or protected, something went wrong and it's not being detected. |
Looking closer - I don't think that's really the problem. The subscribe could fail, but I'm not sure why it would. I'll add a check for that. But the connection and publish are obviously working otherwise the server wouldn't be running the command. Can you run |
Ok I feel dumb now. The command line interface requires that you have insteon-mqtt running. I was stopping it to run the commands in the same terminal window. Everything is working the way it should but it might be worth documenting this. |
I actually thought of that, but thought better of it. No worries, thanks for helping to bug test stuff. I did just push a few more commits that caught a complicated glitch. You may want to update to that. I have found all of the bugs I know of now. |
I made some changes and ran a few dry run syncs and everything looks good. It does seem like you need to restart insteon-mqtt for the scenes.yaml changes to be picked up. Is that expected? This project is really impressive and the scene syncing means that I can finally get away from ISY. Over the next few days I will deploy this and switch Home Assistant over. Once I have my ISY removed I will try actually syncing some scene changes. |
You shouldn't need to restart. However, if you were running as a dry_run, it obviously wouldn't do anything. Was there something in particular you noticed that seemed to require a restart? |
Here is what I am doing
|
Hmm, thank you for the description. Let me try and run that on my end and figure out what the issue is. |
OK, I am seeing an issue too. Although, mine doesn't change when I restart. I will try and poke at this while watching football tomorrow. Should be an easy fix, it was working at some point. |
Sorry I just caught something in your comment. When you say 4. Modify a scene. Are you editing the scenes.yaml file? If so, you are correct, it only reads this on loading the program any changes you make would be overwritten by an import_scenes command. I suppose we could add a command to allow reloading? That wouldn't be hard. Is that something we think people would do? I suppose people may edit the file when they add a new device. |
That is correct. It probably does make sense to have a way to reload the scenes file. That seems to be the way Home Assistant went with config files. |
OK. I rechecked sync and import-scenes. I am seeing the behavior I expect. If you:
You should only see Sync changes that are expected. If your network is healthy, you won't see any sync changes. A few healing changes that I have found on my network:
I added #175 as a feature request for reloading the config file. We can discuss issues with that there. This branch is already 75+ commits, I would rather avoid adding anymore enhancements to the branch. |
Thanks @krkeegan. If anyone finds any problems with scene management, please open a new issue. |
I think about the last thing preventing me from fully switching over to insteon-mqtt is support for creating links and scenes between Insteon devices programmatically (e.g. through config.yaml). This effectively allows the creation of 3-way and 4-way switches from Insteon devices. For example this code (below) in Mr. House links together 3 switches in the kitchen to act as a 3-way switch circuit and 4 switches in the living room to act as a 4-way switch circuit controlling a lamplinc as well as the devices attached to those switches. Finally, these are also setup as virtual PLM buttons which can control the scenes as well.
In this the "name " column refers to the Insteon scene name (used internally by Mr House only), the "device" column refers to the device name in the Mr. House configuration (which then provides the hex address), the "cont" column specifies whether the device should be a controller for the scene, the "resp" whether the device should be a responder in the scene, etc.
The text was updated successfully, but these errors were encountered: