Insteon: PLM Parser Should: 1 Log Dropped Messages and 2 Decode All Messages

[krkeegan]

Two issues:

1. Insteon_PLM silently dumps identical consecutive messages received in the same pass (see around line 600). If possible a log message should be made when this occurs. There is a potential, that given how the parser functions, that this may not be possible.
2. If multiple messages are received in the same line, the level 4 message decoder only processes the first message. This is not ideal. The decoder should decode all messages.

[peloy]

Hi Kevin,

On 05/15/2013 02:11 PM, Kevin Robert Keegan wrote:

Two issues:

1. Insteon_PLM silently dumps identical consecutive messages received in
   the same pass (see around line 600). If possible a log message should be
   made when this occurs. There is a potential, that given how the parser
   functions, that this may not be possible.

I am wondering about your last comment regarding possibility of doing
this... what do you have in mind that I am missing?

The obvious approach to producing a log message for this situation (my
dumb patch below) seems to be working for me:

05/15/13 03:21:20 PM [Insteon_PLM] DEBUG3: Received PLM raw data:
02501419e9000001cb110102501419e9000001cb1101
05/15/13 03:21:20 PM [Insteon_PLM] DEBUG4: Milliseconds 900.35
PLM Command: (0250) insteon_received
From Address: 14:19:e9
To Address: 00:00:01
Message Flags: cb
Message Type: (110) All-Link Broadcast Message
Message Length: (0) Standard Length
Hops Left: 2
Max Hops: 3
Insteon Message: 1101
Cmd 1: (11) ALL-Link Recall
Cmd 2: 01

05/15/13 03:21:20 PM [Insteon::BaseInterface] DEBUG3: Message received
with 2 hops left, delaying next transmit by 250 milliseconds to avoid c
ollisions.
05/15/13 03:21:20 PM [Insteon::BaseInterface] Received message from:
$garage_motion; command: on; type: alllink; group: 01
05/15/13 03:21:20 PM [Insteon::BaseObject] DEBUG3: Adding hop count of 1
to hop_array of $garage_motion
05/15/13 03:21:20 PM [Insteon::MotionSensor]
$garage_motion::set_receive(on, $garage_motion)
05/15/13 03:21:20 PM [Insteon_PLM] DEBUG4: New parsed data
(02501419e9000001cb1101) is the same as the previous parsed data;
assuming a duplicate message and ignoring new parsed data.

I guess another approach is to not do anything here and let the new
duplicate-catching infrastructure make the catch later when it runs, but
it seems to me like we have nothing to lose (and performance to gain) by
dropping the duplicate here.

Cheers,

Eloy Paris.-

---

--- a/lib/Insteon_PLM.pm
+++ b/lib/Insteon_PLM.pm
@@ -598,7 +598,15 @@ sub _parse_data {
{
#ignore blanks.. the split does odd things
next if $parsed_data eq '';

•            next if $previous_parsed_data eq $parsed_data; # guard 
  

  against repeats

•           if ($previous_parsed_data eq $parsed_data)
  

•           {
  

•               # guard against repeats
  

•               &::print_log("[Insteon_PLM] DEBUG4: New parsed data ("
  

•                            . "$parsed_data) is the same as the 
  

  previous "

•                            . "parsed data; assuming a duplicate 
  

  message "

•                            . "and ignoring new parsed data.") if 
  

  $main::Debug{insteon} >= 4;

•               next;
  

•           }
            $previous_parsed_data = $parsed_data; # and, now 
  

  reinitialize

[krkeegan]

The parsing function is somewhat bizarre and is not a type of coding I had ever seen before. I say that it might not be possible, because I wonder if the parser itself actually makes duplicate messages and that by adding a log message we would end up with a lot of false positive log messages.

I am doing a poor job of explaining this, a quick test will certainly determine whether it would work or not.