Re: [mh] insteon branch reload code problem

[Gregg L. <gr...@li...>]

On 1/20/2011 1:00 AM, Eloy Paris wrote:

[... snip ...]
> after reach code reload, the number of devices in the output
> increments by the number of devices I have.

> I have seen a few times now that MisterHouse ceases to control my
> devices after code reloads and I think that this issue may have
> something to do with it. After all, it must be very confusing to the
> code to have duplicate elements in the objects array.

Very good catch; and, you're right--things will become confused.

> So, to avoid adding duplicate devices to the objects array, should we
> not compare the device addresses instead of actual objects; i.e.
> something like:
>
> Index: ../Insteon.pm
> ===================================================================
> --- ../Insteon.pm	(revision 1815)
> +++ ../Insteon.pm	(working copy)
> @@ -438,7 +438,7 @@
>
>       if (ref $$self{objects}) {
>          for (my $i = 0; $i<  scalar(@{$$self{objects}}); $i++) {
> -         if ($$self{objects}->[$i] eq $p_object) {
> +         if ($$self{objects}->[$i]->{device_id} eq
> $p_object->{device_id}) {
>                splice @{$$self{objects}}, $i, 1;
>                return 1;
>             }

Conceptually, that's a reasonable approach.  But, what I think we want 
to do is: 1) retain the existing compare (as it is still valid and 
useful) and 2) implement an "equals" method in the BaseObject class 
(e.g., $p_object->equals($$self{objects}->[$i])).  The equals method 
would do both the deviceid and group comparison as applicable.  By 
implementing an equals method, then we don't start spreading awareness 
of equality internal to an object into other parts of the code.

I'll work on both today and commit.

> Also, there is an InsteonManager::remove_all_items() method that does
> what the name suggest. However, the call "$_->untie_items($self)" in the
> foreach loop is commented out. This makes me wonder -- are there any
> other things that need to be done besides just removing elements of
> objects (in the case we're deleting specific elements), or removing
> entirely the objects list (in the case we're removing everything)?

That commented line is just leftover from the source copy and paste.  It 
can be ignored.  I'm pretty sure there's not anything else needed.

Thanks for the keen eyes!

Gregg