Minor cleanup and isdimmable fix

lib/AlexaBridge.pm

@@ -242,7 +242,7 @@ When the above is said to the Echo, it first gets the current state, then subtra
         #Type set - this is when alexa is asked to turn this device on/off so the state should be set to $state or your action should be run.
 
         if ($type eq 'state') {
-                if ( $light1->can('state_level') ) {
+                if ( $light1->can('is_dimmable')  &&  $light1->is_dimmable ) {
                         return (state $light1).','.$light1->level; # Example returning state and level. NOTE: Ensure state is NOT numeric
                         return $light1->level; # Example returning level only.
                 }
@@ -1036,7 +1036,7 @@ sub get_set_state {
             my $type  = $object->get_type();
             my $debug = "[Alexa] Debug: get_set_state (actual object state: $cstate) - (object type: $type) - ";
             my $return;
-            if ( $object->can('state_level') ) {
+            if ( $object->can('is_dimmable')  &&  $object->is_dimmable ) {
                 my $l = $object->level;
                 $l =~ s/\%//;
                 if ( $l =~ /\d+/ ) {
@@ -1056,7 +1056,7 @@ sub get_set_state {
             return $return;
         }
         elsif ( $action eq 'set' ) {
-            if ( $object->can('state_level') && $state =~ /\d+/ ) { $state = $state . '%' }
+            if ( $object->can('is_dimmable') && $object->is_dimmable && $state =~ /\d+/ ) { $state = $state . '%' }
             &main::print_log("[Alexa] Debug: setting object ( $realname ) to state ( $state )\n") if $main::Debug{'alexa'};
             if   ( lc($type) =~ /clipsal_cbus/ ) { $object->$sub( $state, 'Alexa' ) }
             else                                 { $object->$sub( $state, 'Alexa' ) }

lib/AoGSmartHome_Items.pm

@@ -222,7 +222,7 @@ sub set_state {
     my $sub      = $self->{'uuids'}->{$uuid}->{'sub'};
     my $statesub = $self->{'uuids'}->{$uuid}->{'statesub'};
 
-    # ???
+    # Map state if there is a mapping defined
     $state = $self->{'uuids'}->{$uuid}->{ lc($state) } if $self->{'uuids'}->{$uuid}->{ lc($state) };
 
     print STDERR "[AoGSmartHome] Debug: set_state(uuid='$uuid', state='$state', name='$name' realname='$realname' sub='$sub')\n"
@@ -255,8 +255,7 @@ sub set_state {
         my $mh_object = ::get_object_by_name($realname);
         return undef if !defined $mh_object;
 
-	if ( ($mh_object->isa('Insteon::DimmableLight')
-	      || $mh_object->can('state_level') ) && $state =~ /\d+/ ) {
+	if ( $mh_object->can('is_dimmable' && $mh_object->is_dimmable && $state =~ /\d+/ ) {
 	    $state = $state . '%';
 	}
 
@@ -334,6 +333,7 @@ sub new {
     if ( -e $file ) {
         my $restoredhash = retrieve($file);
         $self->{idmap} = $restoredhash->{idmap};
+	# delete $self->{idmap}->{objects}->{''};
 
         if ( $main::Debug{'aog'} ) {
             print STDERR "[AoGSmartHome] Debug: dumping persistent IDMAP:\n";
@@ -363,6 +363,11 @@ sub add {
     my ( $self, $realname, $name, $sub, $on, $off, $statesub, $dev_properties ) = @_;
     my ($type, $room); # AoG Smart Home Provider device properties
 
+    if( !$realname ) {
+	&main::print_log("[AoGSmartHome] Realname must be specified for persistent idmap; ignoring AoG item.");
+	return;
+    }
+
     if ( !$name ) {
         $name = $realname;
         $name =~ s/\$//g;
@@ -456,8 +461,7 @@ EOF
 	    if( !ref $mh_object ) {
 		&main::print_log("[AoGSmartHome] '$self->{'uuids'}->{$uuid}->{'realname'} is not found.");
 	    }
-	    elsif ($mh_object->isa('Insteon::DimmableLight')
-		|| $mh_object->can('state_level') ) {
+	    elsif( $mh_object->can('is_dimmable')  &&  $mh_object->is_dimmable ) {
 		$response .= <<EOF;
      "action.devices.traits.Brightness",
 EOF
@@ -688,8 +692,7 @@ EOF
 	# If the device is dimmable we provided the "Brightness" trait, so we
 	# have to supply the "brightness" state.
 	my $mh_object = ::get_object_by_name($self->{'uuids'}->{$uuid}->{'realname'});
-	if ($mh_object->isa('Insteon::DimmableLight')
-	    || $mh_object->can('state_level') ) {
+	if( $mh_object->can('is_dimmable')  &&  $mh_object->is_dimmable ) {
 
 	    # INSTEON devices return "on" or "off". The AoG "Brightness" trait
 	    # expects needs "100" or "0", so we adjust here accordingly.

lib/EIB_Items.pm

@@ -725,6 +725,17 @@ sub set_receive {
     $self->SUPER::set_receive( $newstate, $set_by, $target );
 }
 
+=item C<is_dimmable()>
+
+Returns whether object is dimmable.
+
+=cut
+
+sub is_dimmable {
+    my ( $self ) = @_;
+    return 1;
+}
+
 =item C<state_level>
 
 return 'on', 'off' or 'dim', depending on current setting (as obtained from "value" sub-item)

lib/Generic_Item.pm

@@ -254,6 +254,7 @@ sub _set_process {
 
     # Override any set_by_timer requests
     if ( $$self{timer} ) {
+	print "deleting timer due to set call\n" if $::Debug{set};
         &Timer::unset( $$self{timer} );
         delete $$self{timer};
     }

lib/Insteon/BaseInsteon.pm

@@ -30,6 +30,7 @@ package Insteon::BaseObject;
 
 use strict;
 use Insteon::AllLinkDatabase;
+use Data::Dumper;
 
 @Insteon::BaseObject::ISA = ('Generic_Item');
 
@@ -372,6 +373,8 @@ sub set {
     }
 
     if ( $self->_is_valid_state($p_state) ) {
+	&::print_log( "[Insteon::BaseObject] set called on $$self{object_name} valid state: $p_state $$self{device_id}:$$self{m_group}" )
+           if $self->debuglevel( 1, 'insteon' );
 
         # always reset the is_locally_set property unless set_by is the device
         $$self{m_is_locally_set} = 0 unless ref $p_setby and $p_setby eq $self;
@@ -470,6 +473,11 @@ sub set_receive {
           if $self->debuglevel( 1, 'insteon' );
     }
     else {
+        ::print_log( "[Insteon::BaseObject] SUPER::set "
+              . $p_state
+              . " state command for "
+              . $self->get_object_name )
+          if $self->debuglevel( 1, 'insteon' );
         $$self{set_milliseconds} = $curr_milli;
         $self->level($p_state) if $self->can('level');    # update the level value
         $self->SUPER::set( $p_state, $p_setby, $p_response );
@@ -478,12 +486,13 @@ sub set_receive {
 
 =item C<set_with_timer(state, time, return_state, additional_return_states)>
 
-NOTE - This routine appears to be nearly identical, if not identical to the
-C<Generic_Item::set_with_timer()> routine, it is not clear why this routine is
-needed here.
-
 See full description of this routine in C<Generic_Item>
 
+    NOTE:  This timer functionality is required here because the Generic_Item timer
+           is reset by Generic_Item set calls, and the set call for the Generic_Item
+	   in this case is delayed until the acknowledgement is received from the insteon device.
+	   ie set( 'on~10~off' ) would reset the Generic_Item timer when the ack for the 'on' is received.
+
 =cut
 
 sub set_with_timer {
@@ -550,12 +559,11 @@ sub derive_message {
     my $message;
 
     if ( $self->isa("Insteon::BaseController") ) {
-
         # only send out as all-link if the link originates from the plm
         if ( $self->isa("Insteon::InterfaceController") ) {    # return the size of the command stack
             $message = new Insteon::InsteonMessage( 'all_link_send', $self );
         }
-        elsif ( $self->is_root ) {                             # return the size of the command stack
+        elsif ( $self->is_root ) {
             $message = new Insteon::InsteonMessage( 'insteon_send', $self );
         }
         else {
@@ -566,6 +574,7 @@ sub derive_message {
         $message = new Insteon::InsteonMessage( 'insteon_send', $self );
     }
 
+
     if ( !( defined $p_extra ) ) {
         if ( $command eq 'on' ) {
             if ( $self->can('local_onlevel') && defined $self->local_onlevel ) {
@@ -618,6 +627,7 @@ sub derive_message {
     }
 
     $message->command($command);
+
     return $message;
 }
 
@@ -1117,7 +1127,8 @@ sub _process_command_stack {
         ::print_log( "[Insteon::BaseObject] " . $self->get_object_name . " is deaf and not currently awake. Queuing commands" . " until device wakes up." );
     }
     else {
-        #		&::print_log("[Insteon_Device] " . $self->get_object_name . " command queued but not yet sent; awaiting ack from prior command") if $self->debuglevel(1, 'insteon');
+        ::print_log("[Insteon_Device] " . $self->get_object_name . " command queued but not yet sent; awaiting ack from prior command")
+	  if $self->debuglevel(1, 'insteon');
     }
 }
 
@@ -3106,7 +3117,7 @@ they will be used, otherwise 100% .1s will be used.
 sub add {
     my ( $self, $obj, $on_level, $ramp_rate ) = @_;
     if ( ref $obj
-        and ( $obj->isa('Light_Item') or $obj->isa('Insteon::BaseDevice') ) )
+        and ( $obj->isa('Light_Item') or $obj->isa('Insteon::BaseDevice') or $obj->isa('Insteon::InterfaceController') ) )
     {
         if ( $$self{members} && $$self{members}{$obj} ) {
             print "[Insteon::BaseController] An object (" . $obj->{object_name} . ") already exists " . "in this scene.  Aborting add request.\n";
@@ -3260,6 +3271,13 @@ sub sync_links {
         $tgt_ramp_rate =~ s/(\d)s?/$1/;
         my $resp_aldbkey = $member_root->_aldb->get_linkkey( $insteon_object->device_id, $self->group, '0', $member->group );
 
+        # skip scene responders
+        if ( $member->isa('Insteon::InterfaceController') ) {
+            ::print_log("[Insteon::Sync_Links] Skipping link to scene $member_name.")
+              if $insteon_object->debuglevel( 2, 'insteon' );
+	    next MEMBER;
+	}
+
         # If this is an attempt to create a link between two objects on the same
         # device, then skip.  Currently, a KPL can do this with a seperate
         # routine, but IntraDevice links are not allowed by any other known device
@@ -3569,7 +3587,7 @@ sub set_linked_devices {
                 }
                 $member->set_on_state($local_state) unless $link_state eq 'off';
             }
-            elsif ( $member->isa('Insteon::BaseDevice') ) {
+            elsif ( $member->isa('Insteon::BaseDevice') or $member->isa('Insteon::InterfaceController') ) {
 
                 # remember the current state to support resume
                 $$self{members}{$member_ref}{resume_state} = $member->state;

lib/Insteon/Lighting.pm

@@ -303,6 +303,18 @@ sub new {
     return $self;
 }
 
+=item C<is_dimmable()>
+
+Returns whether object is dimmable.
+
+=cut
+
+sub is_dimmable {
+    my ( $self ) = @_;
+    return 1;
+}
+
+
 =item C<local_onlevel(level)>
 
 Sets and returns the local onlevel for the device in MH only. Level is a 

lib/X10_Items.pm

@@ -559,6 +559,17 @@ sub resume {
     return $_[0]->{resume};
 }
 
+=item C<is_dimmable()>
+
+Returns whether object is dimmable.
+
+=cut
+
+sub is_dimmable {
+    my ( $self ) = @_;
+    return 1;
+}
+
 =item C<level>
 
 Returns the current brightness level of the item, 0->100

lib/ZWave_Items.pm

@@ -486,6 +486,17 @@ sub convert_percents {
     return $data;
 }
 
+=item C<is_dimmable()>
+
+Returns whether object is dimmable.
+
+=cut
+
+sub is_dimmable {
+    my ( $self ) = @_;
+    return 1;
+}
+
 sub state_level {
 
     #    print "state level called\n";

lib/read_table_A.pl

@@ -2041,8 +2041,9 @@ sub read_table_finish_A {
         if ( $objects{$scene} ) {
 
             #Since an object exists with the same name as the scene,
-            #make it a controller of the scene, too. Hopefully it can be a controller
+            #make it a controller and responder of the scene, too. Hopefully it can be a controller
             $scene_build_controllers{$scene}{$scene} = "1";
+            $scene_build_responders{$scene}{$scene} = "1";
         }
 
         #Loop through the controller hash