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

Add Scene Management Functionality #179

Merged
merged 85 commits into from
Feb 15, 2020
Merged

Add Scene Management Functionality #179

merged 85 commits into from
Feb 15, 2020

Conversation

krkeegan
Copy link
Collaborator

This adds the following new Features:

  • Adds an optional scenes.yaml file. The location of which is defined in config.yaml. This file can be used to define scenes which will be synced to the user's devices.
  • Scenes are defined in the scenes.yaml file using a user friendly format

Syncing:

  • Scenes defined in the scenes.yaml file can be sync to individual devices using the 'sync' command or all devices using the 'sync-all' command.
  • Options to perform a 'dry-run' which only shows the changes that would be made to the devices.
  • Automatically numbering modem scenes. The user is not required to, and is recommended not to, specify a group number on the modem.

Importing:

  • Users can import scenes defined on their devices into the scenes.yaml file by running 'import-scenes' on an individual device or 'import-scenes-all' for all devices.
  • The order and comments contained within the scenes.yaml file will be preserved as much as possible when importing. (this is safe for small imports, but significant imports will obviously lead to significant changes in the file)

Other:

  • Modem scenes can have a name, which can be used to command them rather than using their group numbers.
  • Tons of documentation (but we can always use more). Please see config.yaml, scenes.yaml, Scene Management, and Mqtt Overview
  • Unit tests (again we could use more)

Known Issues:

  • The scene created by an IOLinc is something I don't know how to handle. The Import function will import it as normal, and if it is present in the scenes.yaml file it will be handled fine. Maybe when adding this scene we should consider adding it to a scenes.yaml if available?

Unknown Issues:

  • Everything. I have tested this a lot but this s#!t is complicated and makes my brain hurt thinking about it sometimes.

Prudent Advice:

  1. If you put a lot of work into creating a scenes.yaml file, please back it up, it would suck to have it get erased. Otherwise the import functions are safe to run at will
  2. Please utilize the dry-run function of sync. Make sure you know why links are being added or deleted before doing it.

Bug Reporting:

  • Obviously anything that causes a crash please tell me about it.
  • Again, this is complicated, so if you think you see a link being added or deleted that shouldn't please stop and review the database and the scenes.yaml file. If you still think something is wrong please report the bug. In the bug report please provide the relevant scenes.yaml entry and the print-db outputs of the two devices involved in the bug report.
  • Please report bugs to ted's repo here.

Phew, alright good luck!

Fixes #25

TD22057 and others added 30 commits November 19, 2018 21:19
This only deals with the device level db, not the modem yet.

Handles three variations of config file defintions.

Adds the concept of data_1, data_2, and data_3 keys in a definition.
Manually fixed conflicts in:
  insteon_mqtt/Modem.py
	insteon_mqtt/device/Base.py
Used to load and save the scenes.yaml file
Migrating code from prior work.

Adds the !rel_path directive to config to allow for specifying a
relative path in the config file.

Reads scenes from the specified file.  If a modem controller link
is defined without a group ID, one will be assigned.

Defined links are then loaded into the db_config for each device.
Will not remove links associated with join or pair.  Also won't
add them.

We somewhat blindly handle pair, since we don't have any list of
available groups for each device.  This could be fixed by adding
in a static list to each object if necessary.

Had to change passed object to the device rather than the db.
This enables us to use methods on the device, such as find.
Enables running on the entire network.

Decrease verbosity of no changes.
Adds or deletes links from the device as defined in scenes.
Ensures that dry_run always matches what would happen.

Also decreases namespace pollution.

Dry_Run is the default.  A user has to specify dry_run=false for
an actual sync to occur.
Print log calls from within CommandSeq so messages appear in
logical order for user.

This required enabling a synchronous function such as LOG.ui
which does not accept an on_done argument to be called by
CommandSeq.

Also run diff after refresh.  We want to perform diff with most
current data.
It is necessary for the db object to be able to make calls on
the base object.

Plus when doing an import, we would like to be able to set the
db_config as the rhs.
Only added to devices currently, and does not save results to disk.

This code is complicated and rather spaghetti like.  This is in
part because we allow a huge amount of latitude for the users
to specify scenes.

This process is undoubtedly complex, but the concept is that
users will rarely use this feature.
Don't try and import these into the Scenes definition.
Duplicates the functions found on the devices.
Just a sequence initiated by the Modem Object
Scene will be imported with address only.
Redesigned Scenes Class to use SceneEntry Classes for each
individual scene and SceneDevice Classes for each device
entry in a scene.

Adds a lot more code, but seems to be much more readable and
logical.
Linking is Hard!

Fix some confusion about what group and data values should be
pushed to which link

Remove unnecessary _load_scenes() command that is no longer used.
Not sure how this got messed up.
Define a weak search that doesn't check for group matches
Wait to do until save, this is moderately faster.

Compress N-Way scenes.
For the most part, this can only happen with KPL devices.  Imagine
a single Modem scene turning on all of the buttons on a KeypadLinc

Streatmlined populate scenes to have two less loops.

Updated tests to match new design
SceneManager is a better description.

Add code documentation to Scenes.py
The fanlinc is similar to a keypadlinc in that it has multiple
groups and some dimmer features.

Removed a bunch of unnecessary documentation in the link_data
functions.  Much of it was repetetive.
Write a small dissertation on how it works.
New Feature:
Allow users to control the modem virtual scenes via a name defined
in the scenes.yaml file.
to Stack

Enables returns back to the main loop in between each function call.
Splitting into multiple calls for better handling by Stacks.

No need to test items less than or equal to themselves since that
test has already been done.
Used the wrong address.

This is really complicated, everytime I touch something I seem to
make a bug.
Fixes some esoteric bugs.

Link_defaults for a multigroup device will report all groups as
being default since we feed in the group number to generate the
default.  This fixes it, so that only group 0 or 1 devices are
treated as default.

Second, init the group value earlier, as it can affect the
link_data names returned.  Such as for a fanlinc where there is
a ramp_rate for the light, but not for the fan.
The comparision test was backwards.
…etter

Adding Log messages to the stack on CommandSeq was a bad idea.  Any error
in executing a function that threw a TypeError would cause the function
to get exectuted again.

In order to keep sync messages displayed near the approriate events
needed to wrap the add and delete calls in functions that write the
log messages.

Also fixed a glitch wherein a DB refresh was asked for, but the
DB diff was calculated prior to receiving the refresed db data.
The loading of the program still halts.  I think this is the right
answer.  I can't think of a way to handle this and continue that
doesn't cause potentially more issues that are harder for the user
to diagnose later.
@TD22057
Copy link
Owner

TD22057 commented Jan 18, 2020

Fantastic! Thanks for doing this - that's a huge amount of work. I'll take a look this weekend and get it merged.

Remotes need multigroup support like KPL for the link_data.

Small error in the data_3 value on multigroup controller entries
the data_3 is the group controller.
@TD22057 TD22057 merged commit 16cbc1b into TD22057:dev Feb 15, 2020
@krkeegan krkeegan deleted the scene_sync branch December 11, 2020 00:12
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.

2 participants