X10/Insteon: Remove Line Which Attempted to Add the X10 Device to the PLM #322
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Symptoms: the PLM states parameter contained the entire hashes of any X10 devices. This made get_states useless on the PLM (it should return an empty array). Instead it returned a bunch of garbage. This further lead to crashed in the json and xml servers.
Cause: the $interface->add($X10_Object) command was calling the add subroutine in Device_Item. I am not even sure how the inheritance allowed this to happen. But this routine expected a list of states to be passed as the first parameter not an object.
Solution: Remove the offending call to add. It is not clear why this was used (the commit notes demonstrate that this was added when support for X10 over Insteon was added). Perhaps the drafter intended to add the X10 device as a member of the PLM? However, the PLM itself has no members, PLM scenes are allowed to have members. Alternatively, perhaps the drafter intended to add the object to the list maintained by InsteonManager? Adding some code to the X10_Item would allow this to work in theory, however it isn't clear to me why you would want this. The list is maintained so that the insteon_manager can assist in link management, something not done by X10 devices. We have also been operating without adding the X10 items to the object list for quite some time without incident. Indeed no current calls to find_members would even return an X10 device as they all specifiy Insteon packages. Finally, perhaps this is some vestige of the old Insteon code.
Removing the offending line solves the issue, and does not appear to have any adverse effects.