Insteon: Pass Message Clearing Decision to on_read_write_aldb

on_read_write_aldb now returns a 1/0 corresponding to whether the current message should be cleared. When a bad message arrived, on_read_write_aldb attempted to requeue the message that was currently pending. However, _process_message did not clear the pending message until after this routine was run. As a result, a new message was not queued because it was duplicative, but then the current message was cleared. This resulted in stalling the message queue. Fixes bug hollie#258
krkeegan · Sep 27, 2013 · 1590563 · 1590563
1 parent e87291d
commit 1590563
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 23 deletions.
diff --git a/lib/Insteon/AllLinkDatabase.pm b/lib/Insteon/AllLinkDatabase.pm
@@ -2127,14 +2127,16 @@ Called as part of any process to read or write to a device's ALDB.
 sub on_read_write_aldb
 {
 	my ($self, %msg) = @_;
-
+	my $clear_message = 1; #Default Action is to Clear the Current Message
 	&::print_log("[Insteon::ALDB_i2] DEBUG3: " . $$self{device}->get_object_name
 		. " [0x" . $$self{_mem_msb} . $$self{_mem_lsb} . "] received: "
 		. lc $msg{extra} . " for _mem_activity=".$$self{_mem_activity}
 		 ." _mem_action=". $$self{_mem_action}) if  $main::Debug{insteon} >= 3;
 
 	if ($$self{_mem_action} eq 'aldb_i2read')
 	{
+		#This is an ACK. Will be followed by a Link Data message, so don't clear
+		$clear_message = 0;
 		#Only move to the next state if the received message is a device ack
 		#if the ack is dropped the retransmission logic will resend the request
 		if($msg{is_ack}) {
@@ -2156,25 +2158,28 @@ sub on_read_write_aldb
 			&::print_log("[Insteon::ALDB_i2] DEBUG3: " . $$self{device}->get_object_name
 				. " [0x" . $$self{_mem_msb} . $$self{_mem_lsb} . "] received duplicate ack. Ignoring.")
 				if  $main::Debug{insteon} >= 3;
+				$clear_message = 0;
 		} elsif(length($msg{extra})<30)
 		{
 			&::print_log("[Insteon::ALDB_i2] WARNING: Corrupted I2 response not processed: "
 				. $$self{device}->get_object_name
 				. " [0x" . $$self{_mem_msb} . $$self{_mem_lsb} . "] received: "
 				. lc $msg{extra} . " for " .  $$self{_mem_action}) if  $main::Debug{insteon} >= 3;
 			$$self{device}->corrupt_count_log(1) if $$self{device}->can('corrupt_count_log');
-            #retry previous address again
-			$self->send_read_aldb(sprintf("%04x", hex($$self{_mem_msb} . $$self{_mem_lsb})));
+			#can't clear message, if valid message doesn't arrive
+			#resend logic will kick in
+			$clear_message = 0;
 		} elsif ($$self{_mem_msb} . $$self{_mem_lsb} ne '0000' and 
 				$$self{_mem_msb} . $$self{_mem_lsb} ne substr($msg{extra},6,4)){
 			::print_log("[Insteon::ALDB_i2] WARNING: Corrupted I2 response not processed, "
 				. " address received did not match address requested: "
 				. $$self{device}->get_object_name
 				. " [0x" . $$self{_mem_msb} . $$self{_mem_lsb} . "] received: "
 				. lc $msg{extra} . " for " .  $$self{_mem_action}) if  $main::Debug{insteon} >= 3;
-            $$self{device}->corrupt_count_log(1) if $$self{device}->can('corrupt_count_log');
-			#retry previous address again
-			$self->send_read_aldb(sprintf("%04x", hex($$self{_mem_msb} . $$self{_mem_lsb})));
+			$$self{device}->corrupt_count_log(1) if $$self{device}->can('corrupt_count_log');
+			#can't clear message, if valid message doesn't arrive
+			#resend logic will kick in
+			$clear_message = 0;
 		}
 		elsif ($$self{_stress_test_act}){
 			$$self{_mem_activity} = undef;
@@ -2369,6 +2374,7 @@ sub on_read_write_aldb
 			. ": unhandled _mem_action=".$$self{_mem_action})
 			if $main::Debug{insteon};
 	}
+	return $clear_message;
 }
 
 sub _write_link

diff --git a/lib/Insteon/BaseInsteon.pm b/lib/Insteon/BaseInsteon.pm
@@ -776,14 +776,8 @@ sub _process_message
 			}
 			elsif ($pending_cmd eq 'read_write_aldb') {
                         	if ($msg{cmd_code} eq $self->message_type_hex($pending_cmd)) {
-					if ($self->_aldb && $self->_aldb->{_mem_action} ne 'aldb_i2writeack'){
-						#This is an ACK. Will be followed by a Link Data message
-						$clear_message = 0;
-						$self->_aldb->on_read_write_aldb(%msg) if $self->_aldb;
-					} else {
-						$self->_aldb->on_read_write_aldb(%msg) if $self->_aldb;
-						$self->_process_command_stack(%msg);
-					}
+					$clear_message = $self->_aldb->on_read_write_aldb(%msg) if $self->_aldb;
+					$self->_process_command_stack(%msg) if ($clear_message);
                         	} else {
                         		$corrupt_cmd = 1;
                         		$clear_message = 0;
@@ -877,15 +871,8 @@ sub _process_message
 		$self->request_status($self);
 	} elsif ($msg{command} eq 'read_write_aldb') {
 		if ($self->_aldb){
-			if ($self->_aldb->{_mem_action} eq 'aldb_i2readack'){
-				#If aldb_i2readack is set then this is good
-				$clear_message = 1;
-				$self->_aldb->on_read_write_aldb(%msg);
-				$self->_process_command_stack(%msg);
-			} else {
-				#This is an out of sequence message
-				$self->_aldb->on_read_write_aldb(%msg);
-			}
+			$clear_message = $self->_aldb->on_read_write_aldb(%msg) if $self->_aldb;
+			$self->_process_command_stack(%msg) if($clear_message);
 		}
 	} elsif ($msg{type} eq 'broadcast') {
 		$self->devcat($msg{devcat});