From f314d16ba206e3946e5a04021e45ef916d96134e Mon Sep 17 00:00:00 2001 From: KRKeegan Date: Thu, 14 Feb 2013 21:04:06 -0800 Subject: [PATCH] Remove ACK Hash and Correct for Discrepancy in Documentation for ALL-Link Cleanup Failure Report Misunderstood what would be warrant a NACK in the All Link Status Report. A NACK is only issued if the PLM is interrupted in sending the direct commands. As such ACK HASH is not needed. Instead an ALL-Link Cleanup Failure Report is issued if a device fails to respond. However, the ALL-Link Cleanup Failure Report is incorrectly documented in the Modem guide. The message is only 6 bytes long and byte 3 in the documentation doesn't exist. --- lib/Insteon/BaseInsteon.pm | 9 -------- lib/Insteon/BaseInterface.pm | 26 +++++++++------------- lib/Insteon_PLM.pm | 42 ++++++++++-------------------------- 3 files changed, 21 insertions(+), 56 deletions(-) diff --git a/lib/Insteon/BaseInsteon.pm b/lib/Insteon/BaseInsteon.pm index c054a53b9..74f26c404 100755 --- a/lib/Insteon/BaseInsteon.pm +++ b/lib/Insteon/BaseInsteon.pm @@ -1842,15 +1842,6 @@ sub set { my ($self, $p_state, $p_setby, $p_respond) = @_; - if ($$self{members}) - { - foreach my $member_ref (keys %{$$self{members}}) - { - my $member = $$self{members}{$member_ref}{object}; - &::print_log("[KRK] " . $member->get_object_name); - } - } - my $rslt_code = $self->Insteon::BaseController::set($p_state, $p_setby, $p_respond); return $rslt_code if $rslt_code; diff --git a/lib/Insteon/BaseInterface.pm b/lib/Insteon/BaseInterface.pm index ec0275d89..9057f4e17 100755 --- a/lib/Insteon/BaseInterface.pm +++ b/lib/Insteon/BaseInterface.pm @@ -352,7 +352,8 @@ sub on_standard_insteon_received if ($msg{type} ne 'broadcast') { $msg{command} = $object->message_type($msg{cmd_code}); - &::print_log("[Insteon::BaseInterface] command:$msg{command}; type:$msg{type}; group: $msg{group}") + &::print_log("[Insteon::BaseInterface] Received message from: ". $object->get_object_name + ."; command: $msg{command}; type: $msg{type}; group: $msg{group}") if (!($msg{is_ack} or $msg{is_nack})) and $main::Debug{insteon}; } if ($msg{is_ack} or $msg{is_nack}) @@ -398,28 +399,21 @@ sub on_standard_insteon_received $object = &Insteon::get_object('000000', $msg{extra}); if ($object) { - my %cleanup_msg = ('type' => 'cleanup', - 'group' => $msg{extra}, - 'is_ack' => 1, - 'command' => 'cleanup' - ); # prevent re-processing transmit queue until after clearing occurs $self->transmit_in_progress(1); - # ask the object to process the received message and update its state - $object->_process_message($self, %cleanup_msg); # Don't clear active message as ACK is only one of many if (($msg{extra} == $self->active_message->setby->group)){ &main::print_log("[Insteon::BaseInterface] DEBUG3: Cleanup message received for scene " - . $object->get_object_name . ", which matched active message.") if $main::Debug{insteon} >= 3; - # Add to array + . $object->get_object_name . " from source " . uc($msg{source})) + if $main::Debug{insteon} >= 3; } else { &main::print_log("[Insteon::BaseInterface] DEBUG3: Cleanup message received for scene " - . $object->get_object_name . ", but group/command in recent message " - . $msg{extra}."/".$object->message_type($msg{cmd_code}). " did not match group in " - . "prior sent message group/command " . $self->active_message->setby->group - ."/".$self->active_message->command) if $main::Debug{insteon} >= 3; - # Ignore failed messages, if we don't receive an ACK then we assume it was a NACK. + . $object->get_object_name . ", but group in recent message " + . $msg{extra}. " did not match group in " + . "prior sent message group " . $self->active_message->setby->group) + if $main::Debug{insteon} >= 3; + # Leave object in waiting ACK hash } # If ACK or NACK received then PLM is still working on the ALL Link Command # Increase the command timeout to wait for next one @@ -429,7 +423,7 @@ sub on_standard_insteon_received { &main::print_log("[Insteon::BaseInterface] ERROR: received cleanup message " . "that does not correspond to a valid PLM group. Corrupted message is assumed " - . "and will be skipped!"); + . "and will be skipped! Was group " . $msg{extra}); } } else diff --git a/lib/Insteon_PLM.pm b/lib/Insteon_PLM.pm index be1ad2309..5a8239cf3 100644 --- a/lib/Insteon_PLM.pm +++ b/lib/Insteon_PLM.pm @@ -607,16 +607,16 @@ sub _parse_data { &::print_log("[Insteon_PLM] DEBUG2: ALL-Linking Completed with $link_address ($message_data)") if $main::Debug{insteon} >= 2; $self->clear_active_message(); } - elsif ($parsed_prefix eq $prefix{all_link_clean_failed} and ($message_length == 14)) + elsif ($parsed_prefix eq $prefix{all_link_clean_failed} and ($message_length == 12)) { #ALL-Link Cleanup Failure Report - $self->retry_active_message(); + #$self->retry_active_message(); # extract out the pertinent parts of the message for display purposes # bytes 0-1 - ignore; 2-3 - group; 4-9 device address - my $failure_group = substr($message_data,2,2); - my $failure_device = substr($message_data,4,6); + my $failure_group = substr($message_data,0,2); + my $failure_device = substr($message_data,2,6); &::print_log("[Insteon_PLM] DEBUG2: Received all-link cleanup failure from device: " - . "$failure_device and group: failure_group") if $main::Debug{insteon} >= 2; + . "$failure_device and group: $failure_group") if $main::Debug{insteon} >= 2; } elsif ($parsed_prefix eq $prefix{all_link_record} and ($message_length == 20)) { #ALL-Link Record Response @@ -637,40 +637,20 @@ sub _parse_data { my $cleanup_ack = substr($message_data,0,2); if ($cleanup_ack eq '15') { - my $delay_in_seconds = 1.0; # this may need to be tweaked - &::print_log("[Insteon_PLM] WARN1: Received all-link cleanup failure for current message." - . " Attempting resend in " . $delay_in_seconds . " seconds.") + &::print_log("[Insteon_PLM] WARN1: All-link cleanup failure for scene: " + . $self->active_message->setby->get_object_name . ". Retrying in 1 second.") if $main::Debug{insteon} >= 1; $self->retry_active_message(); # except that we should cause a bit of a delay to let things settle out - $self->_set_timeout('xmit',$delay_in_seconds * 1000); + $self->_set_timeout('xmit', 1000); $process_next_command = 0; } else { my $message_to_string = ($self->active_message) ? $self->active_message->to_string() : ""; - &::print_log("[Insteon_PLM] DEBUG2: Received all-link cleanup success: $message_to_string") - if $main::Debug{insteon} >= 2; - - # attempt to process the message by the link object; this acknowledgement will reset - # the auto-retry timer - if ($self->active_message && $self->active_message->isa('Insteon::InsteonMessage') - &&($self->active_message->command_type == 'all_link_send')) - { - my $group = substr($self->active_message->interface_data,0,2); - my $link = &Insteon::get_object('000000',$group); - if ($link) - { - my %msg = ('type' => 'cleanup', - 'group' => $group, - 'is_ack' => 1, - 'command' => 'cleanup' - ); - $link->_process_message($self, %msg); - } - # only clear the active message if this all_link_clean_status corresponds to a all_link_send message - $self->clear_active_message(); - } + &::print_log("[Insteon_PLM] Received all-link cleanup success: $message_to_string") + if $main::Debug{insteon}; + $self->clear_active_message(); } } elsif (substr($parsed_data,0,2) eq '15')