Merge pull request #328 from mstovenour/fix_responder_data3

Move aldb link data3 logic into Insteon objects

Fix data3 for responder records to PLM scene
Add flexibility to how aldb link data3 values are specified; each Insteon device can specify a unique behavior
Update so that most Insteon objects use group 00 for endpoint 1 responder links (was 01 for all I2CS devices)
Override logic for Insteon::SwitchLincRelay to use group 01 for endpoint 1 responder links for I2CS devices
Note:  Might consider doing the same for data1 and data2 in a future update

lib/Generic_Item.pm

@@ -1352,7 +1352,6 @@ sub debuglevel
  $debug_level = 1 unless $debug_level;
  my $objname;
  $objname = lc $object->get_object_name if defined $object;
- ::print_log("[Generic_Item] debuglevel: Processing debug for object $objname ... " . $main::Debug{$objname}) if $main::Debug{$debug_group} >= 5;
  return 1 if $main::Debug{$debug_group} >= $debug_level;
  return 1 if defined $objname && $main::Debug{$objname} >= $debug_level;
  return 0;

lib/Insteon/AllLinkDatabase.pm

@@ -488,7 +488,7 @@ sub delete_orphan_links
 
  # Initialize Variables
  my ($linked_device, $plm_scene, $controller_object, $link_defined,
- $controller_id, $responder_id, $link_data3, $recip_data3, 
+ $controller_id, $responder_id, $recip_data3, 
  $group_object, $data3_object);
  my $group = lc $$self{aldb}{$linkkey}{group};
  my $is_controller = $$self{aldb}{$linkkey}{is_controller};
@@ -536,21 +536,20 @@ sub delete_orphan_links
  my @lights = $member->find_members('Insteon::BaseLight');
  $member = $lights[0] if (@lights); # pick the first
  }
- # For resp, D3 = resp group; For cont, D3 = cont group
- $link_data3 = ($is_controller) ? $group : $member->group;
+#TODO - In the ALDB, the primary key is the combination of device ID "and" group
+#It is possible to link two buttons on a keypad link to the same controller
+#e.g. one button turns on and the other turns off when the controller activates
+#So then we should be checking the group here too. Otherwise when removing links
+#this could inadvertently leave extra links in the ALDB.
  if (lc($member->device_id) eq $responder_id) {
+ #Ask self what we should have in data3
+ #For rspndr, D3 = rspndr group; For ctrlr, D3 = ctrlr group
+ my $link_data3 = $$self{device}->link_data3(($is_controller ? $group : $member->group), $is_controller);
  if ($data3 eq $link_data3){
  $link_defined = 1;
  $recip_data3 = ($is_controller) ? $member->group : $group;
  last MEMBERS;
  } 
- elsif (($link_data3 eq '00' || $link_data3 eq '01') && 
- ($data3 eq '00' || $data3 eq '01' )){
- # Allow for 00 or 01 interchangability
- $link_defined = 1;
- $recip_data3 = ($is_controller) ? $member->group : $group;
- last MEMBERS;
- }
  } 
  }
 
@@ -850,14 +849,7 @@ sub add_link
  }
  my $is_controller = ($link_parms{is_controller}) ? 1 : 0;
 
- # For I2CS devices the default data3 for links is 01
- # For all other devices the default data3 for links is 00
- my $data3_default = '00';
- if ($insteon_object->can('engine_version') && $insteon_object->engine_version eq 'I2CS') {
- $data3_default = '01';
- }
- my $data3 = ($link_parms{data3}) ? $link_parms{data3} : '00';
- $data3 = $data3_default if ($data3 eq '00' || $data3 eq '01');
+ my $data3 = $$self{device}->link_data3($link_parms{data3},$is_controller);
 
  # check whether the link already exists
  my $key = $self->get_linkkey($device_id, $group, $is_controller, $data3);
@@ -912,7 +904,8 @@ sub add_link
  {
  &::print_log("[Insteon::AllLinkDatabase] DEBUG2: adding link record to " . $$self{device}->get_object_name
  . " light level controlled by " . $insteon_object->get_object_name
- . " and group: $group with on level: $on_level and ramp rate: $ramp_rate")
+ . " and group: $group with on level: $on_level,"
+ . " ramp rate: $ramp_rate, local load(data3): $data3")
  if $self->{device}->debuglevel(2, 'insteon');
  my ($data1, $data2);
  if($link_parms{is_controller}) {
@@ -971,19 +964,16 @@ sub update_link
  # strip optional s (seconds) to append ramp_rate
  my $ramp_rate = $link_parms{ramp_rate};
  $ramp_rate =~ s/(\d)s?/$1/;
- &::print_log("[Insteon::AllLinkDatabase] updating " . $$self{device}->get_object_name . " light level controlled by " . $insteon_object->get_object_name
- . " and group: $group with on level: $on_level and ramp rate: $ramp_rate") if $self->{device}->debuglevel(1, 'insteon');
  my $data1 = &Insteon::DimmableLight::convert_level($on_level);
  my $data2 = ($$self{device}->isa('Insteon::DimmableLight')) ? &Insteon::DimmableLight::convert_ramp($ramp_rate) : '00';
 
- # For I2CS devices the default data3 for links is 01
- # For all other devices the default data3 for links is 00
- my $data3_default = '00';
- if ($insteon_object->can('engine_version') && $insteon_object->engine_version eq 'I2CS') {
- $data3_default = '01';
- }
- my $data3 = ($link_parms{data3}) ? $link_parms{data3} : '00';
- $data3 = $data3_default if ($data3 eq '00' || $data3 eq '01');
+ my $data3 = $$self{device}->link_data3($link_parms{data3},$is_controller);
+
+ &::print_log("[Insteon::AllLinkDatabase] DEBUG2: updating " . $$self{device}->get_object_name 
+ . " light level controlled by " . $insteon_object->get_object_name
+ . " and group: $group with on level: $on_level,"
+ . " ramp rate: $ramp_rate, local load(data3): $data3")
+ if $self->{device}->debuglevel(2, 'insteon');
 
  $$self{_success_callback} = ($link_parms{callback}) ? $link_parms{callback} : undef;
  $$self{_failure_callback} = ($link_parms{failure_callback}) ? $link_parms{failure_callback} : undef;
@@ -1181,7 +1171,17 @@ sub has_link
  $deviceid = lc $insteon_object->device_id;
  }
  my $key = $self->get_linkkey($deviceid, $group, $is_controller, $data3);
- return (defined $$self{aldb}{$key});
+
+ #Now that we have a linkkey, compare the linkkey's data3 value.
+ my $found = 0;
+ if (defined $$self{aldb}{$key}) {
+ #Ask self what we should have in data3
+ $data3 = $$self{device}->link_data3($data3,$is_controller);
+
+ $found++ if( $$self{aldb}{$key}{data3} == $data3);
+ }
+
+ return ($found);
 }
 
 =back
@@ -2778,7 +2778,17 @@ sub has_link
  my ($self, $insteon_object, $group, $is_controller, $data3) = @_;
  my $key = $self->get_linkkey($insteon_object->device_id, 
  $group, $is_controller, $data3);
- return (defined $$self{aldb}{$key});
+
+ #Now that we have a linkkey, compare the linkkey's data3 value.
+ my $found = 0;
+ if (defined $$self{aldb}{$key}) {
+ #Ask self what we should have in data3
+ $data3 = $$self{device}->link_data3($data3,$is_controller);
+
+ $found++ if( $$self{aldb}{$key}{data3} == $data3);
+ }
+
+ return ($found);
 }
 
 =back

lib/Insteon/BaseInsteon.pm

@@ -2704,6 +2704,40 @@ sub get_voice_cmds
  return \%voice_cmds;
 }
 
+=item C<link_data3>
+
+Returns the data3 value that should be used when creating a link for this device. 
+This sub provides the default for all Insteon devices. Individual device 
+definitions can override this sub to return something unique. This sub was 
+modivated by the need to return unique values for data3 on responder links for 
+group 01. Most Insteon devices will learn 00 for the data3 responder link value, 
+however the In-LineLinc Relay learns 01 when manually linking and requires that 
+value when creating the links programmatically.
+
+=cut 
+
+sub link_data3
+{
+ my ($self, $group, $is_controller) = @_;
+
+ my $link_data3;
+
+ if( $is_controller) {
+ #Default to 01 if no group was supplied
+ #Otherwise just return the group
+ $link_data3 = ($group) ? $group : '01';
+ } else { #is_responder
+ #Default to 00 if no group was supplied
+ $link_data3 = ($group) ? $group : '00';
+
+ #Default for group 01 is data3 == 00
+ #Override this in inerited classes if needed
+ $link_data3 = '00' if( $group eq '01');
+ }
+
+ return $link_data3;
+}
+
 =back
 
 =head2 INI PARAMETERS
@@ -2999,10 +3033,9 @@ sub sync_links
  $$self{sync_queue_failure_callback} = ($failure_callback) ? $failure_callback : undef;
  $$self{sync_queue_failure} = undef;
  my $self_link_name = $self->get_object_name;
- my $insteon_object = $self->interface;
  my $interface_object = Insteon::active_interface();
  my $interface_name = $interface_object->get_object_name;
- $insteon_object = $self->get_root;
+ my $insteon_object = $self->get_root;
  ::print_log("[Insteon::BaseController] WARN!! A device w/ insteon address: " . $self->device_id . ":01 could not be found. "
  . "Please double check your items.mht file.") if (!(defined($insteon_object)));
 

lib/Insteon/Lighting.pm

@@ -626,6 +626,35 @@ sub new
  return $self;
 }
 
+
+=item C<link_data3>
+
+Returns the data3 value that should be used when creating a link for this device. 
+This sub overides the parent class to map group 01 to data3 01. This is required 
+by the I2CS In-LineLinc Relay.
+
+=cut 
+
+sub link_data3
+{
+ my ($self, $group, $is_controller) = @_;
+
+ my $link_data3 = $self->SUPER::link_data3($group, $is_controller);
+
+ if( !$is_controller) { #is_responder
+ #For I2CS devices the default data3 for responder links is 01.
+ #This is to support the I2CS In-LineLinc Relay. There may be more 
+ #permutations of the 00 vs. 01 problem and I1 devices may have 
+ #the same requirement. This code is a work in progress as more 
+ #information is gathered about Relay type devices.
+ if ($self->can('engine_version') && $self->engine_version eq 'I2CS') {
+ $link_data3 = '01';
+ }
+ }
+
+ return $link_data3;
+}
+
 =back
 
 =head2 AUTHOR

lib/Insteon_PLM.pm

@@ -927,6 +927,33 @@ sub firmware {
  return $$self{firmware};
 }
 
+=item C<link_data3>
+
+Returns the data3 value that should be used when creating a link for this device. 
+This sub was modivated by the need to return unique values for data3 on responder 
+links for group 01. The PLM will store the responder's devcat data for controller 
+entries. That's fundamentally hard so just do the same as for other devices for 
+now. Can make this smarter in the future if needed.
+
+=cut 
+
+sub link_data3
+{
+ my ($self, $group, $is_controller) = @_;
+
+ my $link_data3;
+
+ if( $is_controller) {
+ #Default to 01 if no group was supplied
+ #Otherwise just return the group
+ $link_data3 = ($group) ? $group : '01';
+ } else { #is_responder
+ #Default to 01 if no group was supplied
+ $link_data3 = ($group) ? $group : '01';
+ }
+
+ return $link_data3;
+}
 =back
 
 =head2 INI PARAMETERS