AUDIT - sync all links fails

[marcmerlin]

22/03/2013 21:35:41 [Sync all links] Adding $garage2_neon_kpl_gar_outlights to sync queue
22/03/2013 21:35:41 [Sync all links] Adding $garage2_neon_kpl_yard_lights to sync queue
tie_events eval error: Can't call method "health" on an undefined value at ../lib/Insteon.pm line 173, line 11.
at ./mh line 31
main::ANON('Can't call method "health" on an undefined value at ../lib/I...') called at ../lib/Insteon.pm line 169
Insteon::sync_all_links(1) called at (eval 94479) line 1
eval '&Insteon::sync_all_links(1)
;' called at ./mh line 2476
main::check_for_tied_events('Voice_Cmd=HASH(0xb7cf640)') called at ../lib/Generic_Item.pm line 1144
Generic_Item::reset_states2('Voice_Cmd=HASH(0xb7cf640)', 'AUDIT - sync all links', 'web [192.168.205.7]', '') called at ../lib/Generic_Item.pm line 1461
Generic_Item::reset_states called at ./mh line 1499
main::check_for_action called at ./mh line 3296
main::monitor_commands called at ./mh line 6713
Sending 22 bytes of data to localhost port 8084, socket made, data sent
22/03/2013 21:35:50 Received server_data data: name=: action=run arg=

[krkeegan]

This could occur if a device does not have an aldb object. This might happen if a new device is added, and get_engine_version is not run on the device prior to a sync_all_links???

I can try duplicating this tomorrow, but that is only a guess as to how it is occuring. Again a longer log message would likely help here.

[marcmerlin]

I had longer logs in the Email on the list. I didn't repaste everything here, sorry.
This is after a fresh restart.

I can reproduce at will:
25/03/2013 07:48:34 Running: PLM AUDIT - sync all links
25/03/2013 07:48:34 [Sync all links] Starting now!
25/03/2013 07:48:34 [Sync all links] Ignoring links from 'deaf' device: $rlink_blk1_1
25/03/2013 07:48:34 [Sync all links] Ignoring links from 'deaf' device: $rlink_blk1_2
25/03/2013 07:48:34 [Sync all links] Ignoring links from 'deaf' device: $rlink_blk1_3
25/03/2013 07:48:34 [Sync all links] Ignoring links from 'deaf' device: $rlink_blk1_4
25/03/2013 07:48:34 [Sync all links] Ignoring links from 'deaf' device: $rlink_blk1_5
25/03/2013 07:48:34 [Sync all links] Ignoring links from 'deaf' device: $rlink_blk1_6
(...)
25/03/2013 07:48:34 [Sync all links] Adding $mbr_kpl_mbr_outside to sync queue
25/03/2013 07:48:34 [Sync all links] Adding $mbr_kpl_kplC to sync queue
25/03/2013 07:48:34 [Sync all links] Adding $mbr_kpl_crows to sync queue
25/03/2013 07:48:34 [Sync all links] Adding $garage1_neon_kpl_kplA to sync queue
25/03/2013 07:48:34 [Sync all links] Adding $garage1_neon_kpl_garage2_neon to sync queue
25/03/2013 07:48:34 [Sync all links] Adding $garage1_neon_kpl_gar_outlights to sync queue
25/03/2013 07:48:34 [Sync all links] Adding $garage1_neon_kpl_yard_lights to sync queue
25/03/2013 07:48:34 [Sync all links] Adding $garage2_neon_kpl_garage1_neon to sync queue
25/03/2013 07:48:34 [Sync all links] Adding $garage2_neon_kpl_kplB to sync queue
25/03/2013 07:48:34 [Sync all links] Adding $garage2_neon_kpl_gar_outlights to sync queue
25/03/2013 07:48:34 [Sync all links] Adding $garage2_neon_kpl_yard_lights to sync queue
tie_events eval error: Can't call method "health" on an undefined value at ../lib/Insteon.pm line 173, line 11.
at ./mh line 31
main::ANON('Can't call method "health" on an undefined value at ../lib/I...') called at ../lib/Insteon.pm line 169
Insteon::sync_all_links(1) called at (eval 106579) line 1
eval '&Insteon::sync_all_links(1)
;' called at ./mh line 2476
main::check_for_tied_events('Voice_Cmd=HASH(0xbeb76f0)') called at ../lib/Generic_Item.pm line 1144
Generic_Item::reset_states2('Voice_Cmd=HASH(0xbeb76f0)', 'AUDIT - sync all links', 'web [192.168.205.7]', '') called at ../lib/Generic_Item.pm line 1461
Generic_Item::reset_states called at ./mh line 1499
main::check_for_action called at ./mh line 3296
main::monitor_commands called at ./mh line 6713

[marcmerlin]

To get debugging, I did this:
sub sync_all_links
{
my ($audit_mode) = @_;
&main::print_log("[Sync all links] Starting now!");
@_sync_devices = ();
# iterate over all registered objects and compare whether the link tables match defined scene linkages in known Insteon_Links
for my $obj (&Insteon::find_members('Insteon::BaseController'))
{
use Data::Dumper;
my $objname = $obj->{'object_name'};
my $aldb = $obj->_aldb;
&main::print_log("Dumping $objname / $aldb");

It shows that gar_outlights_kpl_garage1_neon has a non defined aldb entry.

25/03/2013 08:32:24 Dumping $garage2_neon_kpl_gar_outlights / Insteon::ALDB_i1=HASH(0xb0f34e4)
25/03/2013 08:32:24 [Sync all links] Adding $garage2_neon_kpl_gar_outlights to sync queue
25/03/2013 08:32:24 Dumping $garage2_neon_kpl_yard_lights / Insteon::ALDB_i1=HASH(0xb0f34e4)
25/03/2013 08:32:24 [Sync all links] Adding $garage2_neon_kpl_yard_lights to sync queue
25/03/2013 08:32:24 Dumping $gar_outlights_kpl_garage1_neon /
tie_events eval error: Can't call method "health" on an undefined value at ../lib/Insteon.pm line 178, line 11.

INSTEON_KEYPADLINCRELAY, 20.FB.30:01, gar_outlights_kpl, All_Lights # vx.x keypadlinc dualband relay

Argh, I found my problem: I forgot to change the config in items.mht for buttons 3,4,5,6 (elsewhere in my file).
This is because I have a split config file to help for things being a bit more readable. My apologies for my mistake.

bad:
INSTEON_KEYPADLINC, 07.DE.8B:03, gar_outlights_kpl_garage1_neon, gar_outlights_kplA|buttons
INSTEON_KEYPADLINC, 07.DE.8B:04, gar_outlights_kpl_garage2_neon, gar_outlights_kplB|buttons
INSTEON_KEYPADLINC, 07.DE.8B:05, gar_outlights_kpl_kplC, gar_outlights_kplC|buttons
INSTEON_KEYPADLINC, 07.DE.8B:06, gar_outlights_kpl_yard_lights, gar_outlights_kplD|buttons

good:
INSTEON_KEYPADLINCRELAY, 20.FB.30:03, gar_outlights_kpl_garage1_neon, gar_outlights_kplA|buttons
INSTEON_KEYPADLINCRELAY, 20.FB.30:04, gar_outlights_kpl_garage2_neon, gar_outlights_kplB|buttons
INSTEON_KEYPADLINCRELAY, 20.FB.30:05, gar_outlights_kpl_kplC, gar_outlights_kplC|buttons
INSTEON_KEYPADLINCRELAY, 20.FB.30:06, gar_outlights_kpl_yard_lights, gar_outlights_kplD|buttons

It would be nice to add something to the parser to make sure that KPLs are consistent, but that's indeed a new issue.

[krkeegan]

It looks like you made two changes:
1.KPL -> KPLRelay
2. Change in Device ID

I don't think that having a KPL versus a KPLRelay would cause any of the
issues you posted. The only problem I would expect is an error relating to
on_level or something similar, but nothing regarding sync or delete links.

The device_id was likely the culprit though. We may not be able to quickly
add code to recover from this error, but I think we can pretty quickly add
a print_log message that explains things better or at least makes this
easier to track down.

On Mon, Mar 25, 2013 at 9:00 AM, Marc MERLIN notifications@github.comwrote:

To get debugging, I did this:
sub sync_all_links
{
my ($audit_mode) = @_;
&main::print_log("[Sync all links] Starting now!");
@_sync_devices = ();

iterate over all registered objects and compare whether the link tables

match defined scene linkages in known Insteon_Links
for my $obj (&Insteon::find_members('Insteon::BaseController'))
{
use Data::Dumper;
my $objname = $obj->{'object_name'};
my $aldb = $obj->_aldb;
&main::print_log("Dumping $objname / $aldb");

It shows that gar_outlights_kpl_garage1_neon has a non defined aldb entry.

25/03/2013 08:32:24 Dumping $garage2_neon_kpl_gar_outlights /
Insteon::ALDB_i1=HASH(0xb0f34e4)
25/03/2013 08:32:24 [Sync all links] Adding
$garage2_neon_kpl_gar_outlights to sync queue
25/03/2013 08:32:24 Dumping $garage2_neon_kpl_yard_lights /
Insteon::ALDB_i1=HASH(0xb0f34e4)
25/03/2013 08:32:24 [Sync all links] Adding $garage2_neon_kpl_yard_lights
to sync queue
25/03/2013 08:32:24 Dumping $gar_outlights_kpl_garage1_neon /
tie_events eval error: Can't call method "health" on an undefined value at
../lib/Insteon.pm line 178, line 11.

INSTEON_KEYPADLINCRELAY, 20.FB.30:01, gar_outlights_kpl, All_Lights # vx.x
keypadlinc dualband relay

Argh, I found my problem: I forgot to change the config in items.mht for
buttons 3,4,5,6 (elsewhere in my file).
This is because I have a split config file to help for things being a bit
more readable. My apologies for my mistake.

bad:
INSTEON_KEYPADLINC, 07.DE.8B:03, gar_outlights_kpl_garage1_neon,
gar_outlights_kplA|buttons
INSTEON_KEYPADLINC, 07.DE.8B:04, gar_outlights_kpl_garage2_neon,
gar_outlights_kplB|buttons
INSTEON_KEYPADLINC, 07.DE.8B:05, gar_outlights_kpl_kplC,
gar_outlights_kplC|buttons
INSTEON_KEYPADLINC, 07.DE.8B:06, gar_outlights_kpl_yard_lights,
gar_outlights_kplD|buttons

good:
INSTEON_KEYPADLINCRELAY, 20.FB.30:03, gar_outlights_kpl_garage1_neon,
gar_outlights_kplA|buttons
INSTEON_KEYPADLINCRELAY, 20.FB.30:04, gar_outlights_kpl_garage2_neon,
gar_outlights_kplB|buttons
INSTEON_KEYPADLINCRELAY, 20.FB.30:05, gar_outlights_kpl_kplC,
gar_outlights_kplC|buttons
INSTEON_KEYPADLINCRELAY, 20.FB.30:06, gar_outlights_kpl_yard_lights,
gar_outlights_kplD|buttons

It would be nice to add something to the parser to make sure that KPLs are
consistent, but that's indeed a new issue.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/136#issuecomment-15402060
.

[mstovenour]

Interesting. It might not be that hard to add logic that requires a new object with group other than 01 to already have a parent 01 device with the same object ID. The new() could just reject this outright. That would cover .mht files as well as user code that tries to create Insteon objects.

Kevin, this is loosely related to your Thermostat design problem. How to specify dependent objects that behave as first class MH objects but can also be identified as part of the parent? The Insteon case above is trivial but if we start looking at more sophisticated GUIs for things like Keypad Links and Thermostats it might make sense to bind these together a bit stronger.

[krkeegan]

It is a similar problem. Keypadlincs and Remotelinc buttons could also be
automatically created (although distinguishing 6 vs 8 button models might
be interesting). IOLincs are other devices that likely would have the same
issue, there are probably more. Automatically spawning them would resolve
issues with user mis-configuration, but would to some degree decrease the
user's configurability.

As for adding logic to new(), I would be careful, if the children are
identified in the config file before the parent, they would never come
into existence even though the parent is properly defined.

Plus, if you don't allow the children to be spawned, what do you do with
them? In Marc's case, simply dropping them would have likely resulted in
more bizarre errors as he has user_code which references them as well as
scenes defined containing them. I think the outcome would have been the
same in that MH would have crashed. If someone can figure out how to
selectively disable objects (which is another open issue) we could copy
that code.

In the meantime, I can add a print_log message to root_device() that will
print before MH crashes. It will help prevent us from chasing down this
error again and may allow some users to catch the problem before seeking
help.

On Mon, Mar 25, 2013 at 11:46 AM, Michael Stovenour <
notifications@github.com> wrote:

Interesting. It might not be that hard to add logic that requires a new
object with group other than 01 to already have a parent 01 device with the
same object ID. The new() could just reject this outright. That would cover
.mht files as well as user code that tries to create Insteon objects.

Kevin, this is loosely related to your Thermostat design problem. How to
specify dependent objects that behave as first class MH objects but can
also be identified as part of the parent? The Insteon case above is trivial
but if we start looking at more sophisticated GUIs for things like Keypad
Links and Thermostats it might make sense to bind these together a bit
stronger.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/136#issuecomment-15413537
.

[marcmerlin]

On Mon, Mar 25, 2013 at 02:03:16PM -0700, krkeegan wrote:

It is a similar problem. Keypadlincs and Remotelinc buttons could also be
automatically created (although distinguishing 6 vs 8 button models might
be interesting). IOLincs are other devices that likely would have the same
issue, there are probably more. Automatically spawning them would resolve
issues with user mis-configuration, but would to some degree decrease the
user's configurability.

As for adding logic to new(), I would be careful, if the children are
identified in the config file before the parent, they would never come
into existence even though the parent is properly defined.

Plus, if you don't allow the children to be spawned, what do you do with
them? In Marc's case, simply dropping them would have likely resulted in
more bizarre errors as he has user_code which references them as well as
scenes defined containing them. I think the outcome would have been the
same in that MH would have crashed. If someone can figure out how to
selectively disable objects (which is another open issue) we could copy
that code.

I believe it would have been more clear because mh would have said that
kpl_button_B doesn't exist instead of crashing on an undefined value without
even telling me the object name :)
That said, short of doing anything better, just having the parser reject
device:02 if device:01 doesn't exist, should be enough. Fail early and fail
often as they say :)

In the meantime, I can add a print_log message to root_device() that will
print before MH crashes. It will help prevent us from chasing down this
error again and may allow some users to catch the problem before seeking
help.

If this catches the different code paths I managed to hit, works for me.

Thanks,

Marc

"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems ....
.... what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/