Thanks Michael, that makes sense.  To my knowledge, nothing really qualifies as as a strict responder only these days.  While I doubt it gets a lot of use, even the set button on an appliance link can be used as a controller.  

I agree your plan of is_dimmable like routines makes the most sense.  In some respects a lot of that work has been started, there are is_responder is_controller and other routines in the base classes, but they don't appear to be used much.  This is however a problem for another day.

Thanks for the help, it sounds like we have pretty close agreement on the design of the voice commands.


On Thu, Jul 18, 2013 at 10:32 PM, Michael Stovenour <michael@stovenour.net> wrote:

Kevin,

I “think” this was based on an assumption in the original Insteon code where we had two device types IPLL (responder) and IPLD (controller).  This assumption was based on the limited functionality of the early Insteon devices.  For instance the SwitchLinc was only a controller.  The LampLinc was only a responder.  However this concept quickly breaks down when we consider that a SwitchLinc is a responder for its attached load but can also be a controller of another SwitchLinc.  I think Gregg simply extended this same concept into OOP when he refactored the Insteon code to be more object oriented.  The result, though, is multiple inheritance for almost all device types, as you’ve pointed out.  In hindsight I think making this distinction was a mistake.

 

This explains why an ApplianceLinc is only a DeviceController (they don’t have buttons).  But it doesn’t explain why a Motion Sensor is a BaseDevice given it can’t be controlled.  But, inheritance from BaseDevice was probably added to pick up some of the more generic functions in that class.  I know that I did the opposite for the LampLinc because it was not initially a DeviceController but I needed the sync_lincs function (and the latest LampLincs have buttons).  The current inheritance model is a classic Big Ball of Mud

 

I wish we had a common base class for all functions and had a simpler inheritance.  The base class would have a bunch of capabilities exposed as check functions that are all set to false.  As objects inherit the base class they could override the capabilities to be true.  I’ve always hated the “isa” checks for specific classes in the Insteon code.  It would be better if there was something less fragile like $object->is_dimable().  For your task each class in the inheritance path could add more voice commands by overriding an add_voice_commands() method and calling super.  A first order refactoring exercise might just be to collapse the two classes together but I can imagine quite a bit of work to find all the unintended consequences.

 

Sincerely,

Michael

 

On July 18, 2013 4:49 PM Kevin wrote:

 

 

I am working on re-arranging the manner in which the Insteon voice commands are generated.

 

I realized, I don't quite understand the distinction between BaseDevice and DeviceController.  For example the ApplianceLinc is a DeviceController only, while the Motion Sensor is a DeviceController first and a BaseDevice second.  Most objects are BaseDevices first and DeviceControllers second.

 

Does anyone remember or know what the distinction between DeviceController and BaseDevice was meant to be?