Skip to content

Commit

Permalink
Fixed error calling method on undefined $self->setby in Message.pm.
Browse files Browse the repository at this point in the history
Fixed error calling undefined method 'level' on RemoteLinc object in BaseInsteon.pm.
  • Loading branch information
cdragon committed Apr 9, 2013
1 parent 07453ad commit 6b8fa95
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
8 changes: 7 additions & 1 deletion lib/Insteon/BaseInsteon.pm
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,13 @@ sub _is_info_request
&::print_log("[Insteon::BaseObject] received status for " .
$self->{object_name} . " with on-level: $ack_on_level%, "
. "hops left: $msg{hopsleft}") if $main::Debug{insteon};
$self->level($ack_on_level); # update the level value

# A RemoteLinc may send Insteon Message: 18b3, meaning "Light Stop Manual Change" aka
# "Stop changing On-Level". But since RemoteLinc doesn't have a level() method defined,
# we will cause a crash here if we don't check if $self->can('level').
if($self->can('level')) {
$self->level($ack_on_level); # update the level value
}
if ($ack_on_level == 0) {

This comment has been minimized.

Copy link
@krkeegan

krkeegan Apr 12, 2013

The issue was fixed in the hollie/master branch by hollie#143

$self->SUPER::set('off', $ack_setby);
} elsif ($ack_on_level > 0 and !($self->isa('Insteon::DimmableLight'))) {
Expand Down
8 changes: 5 additions & 3 deletions lib/Insteon/Message.pm
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,11 @@ sub send
}
}
elsif (defined($$self{no_hop_increase}) && $main::Debug{insteon}){
&main::print_log("[Insteon::BaseMessage] Hop count not increased for "
. $self->setby->get_object_name . " because no_hop_increase flag was set.");
$$self{no_hop_increase} = undef;
if(defined($self->setby)) {
&main::print_log("[Insteon::BaseMessage] Hop count not increased for "
. $self->setby->get_object_name . " because no_hop_increase flag was set.");
$$self{no_hop_increase} = undef;
}

This comment has been minimized.

Copy link
@krkeegan

krkeegan Apr 12, 2013

This issue was fixed in the hollie/master branch by hollie#145

This comment has been minimized.

Copy link
@mstovenour

mstovenour Apr 22, 2013

I am concerned that this code might not do the right thing if debug != Insteon. I see the assignment of undef to no_hop_increase but this will "only" occur if debug == Insteon. Was that intentional?

This comment has been minimized.

Copy link
@dracoventions

dracoventions Apr 22, 2013

Owner

I don't know the code well enough to say what this piece of code is supposed to do or if it should only be run when $main::Debug{insteon} is true. My instinct would be that the no_hop_increase flag should be applied whether or not debug insteon is on, but I could easily be wrong. My commit was intended to fix a crash I encountered where $self->setby was undefined at this point. As such, the hollie#145 fix krkeegan references would also crash when $self->setby is undefined, and this commit of mine would not prevent that crash in the 145 fix.

This comment has been minimized.

Copy link
@krkeegan

krkeegan Apr 22, 2013

The fix in hollie#145 does check if setby is defined as it requires that setby be a Insteon::BaseObject. (see line 120)

This comment has been minimized.

Copy link
@dracoventions

dracoventions Apr 22, 2013

Owner

The only thing I see in hollie#145 is the following change:

    elsif (defined($$self{no_hop_increase}) && $main::Debug{insteon}
                          && $self->setby->isa('Insteon::BaseObject')){

but if $self->setby is undefined, wouldn't calling $self->setby->isa create a crash calling isa on an undefined value?

This comment has been minimized.

Copy link
@krkeegan

krkeegan via email Apr 22, 2013

This comment has been minimized.

Copy link
@krkeegan

krkeegan Apr 23, 2013

OK, hollie#166 should fix this.

}
}

Expand Down

0 comments on commit 6b8fa95

Please sign in to comment.