Fix bug when both showOffline and showUnknown is enabled

[O5ten]

I have a lot of machines in the house aswell as airbnb-guests coming and going with devices so i want to keep track of who is in and not. With this module i was expecting that both showOffline and showUnknown works together to show offline mac-adresses aswell as unknown machines.

So i noticed that it wasn't, so if i turned off showing offline units with showOffline: false then mac-adresses for unknowns were showing up. So i started digging and it turned out that the part that was setting the offline-status were only mapping through the configured devices. It saved only them when creating the new state if showOffline was enabled. The same behavior can still be enabled but by using showUnknown: false of course.

Instead we now add the missing configured devices to the nextState and loop over that collection instead.This allows us to have both settings enabled at the same time as in the example below.

Example with one machine Cybercom turned off and one machine unconfigured

[O5ten]

@ianperrin no longer actively maintaining the project perhaps?

[ianperrin]

@O5ten - @E3V3A and myself have put a fair amount of effort into the module recently to restore and improve functionality, so it’s definately not abandoned. However, given other commitments I cannot continually work on the module so efforts are sporadic.

Thanks for your pull request. Can you confirm you have tested your changes with various combinations of config options using both MAC addresses and IP addresses?

[O5ten]

Yes, i have tested the various combinations of showOffline and showUnknown with devices defined with MacAddress configuration as that is the only type of device that is affected by those configuration options.

I'm fairly confident that i haven't broken anything as this is used and inspected daily in my own mirror. But then again, it's hard to be sure without proper tests. :)

[ianperrin]

Great, thanks.

If you can give me sometime to test the changes with ipAddresses before accepting the pull request, that would be appreciated.

If you want to write some tests...

On 10 Jun 2018, at 20:04, Mikael Östberg [email protected] wrote:

Yes, i have tested the various combinations of showOffline and showUnknown with devices defined with MacAddress configuration as that is the only type of device that is affected by those configuration options.

I'm fairly confident that i haven't broken anything as this is used and inspected daily in my own mirror. But then again, it's hard to be sure without proper tests. :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[O5ten]

No problem, i'll look into how testing is intended to be made in MagicMirror and give it a go.

[ianperrin]

@O5ten - I haven't found a common testing approach for MM modules. However I have now set up Travis CI in the master branch. It's currently failing due to a number of lifting issues but the concept is there certainly to test the node helper file. Feel free to take a look.

[E3V3A]

@ianperrin @O5ten Hi Guys! Sorry I've been too busy lately, with no time at all for MM stuff. Not sure how long this will be until I can get a chance to have look at this again. This PR look like a good improvement though... but need testing -- of course.

[O5ten]

I'll just close this as 6 years seems a bit excessive for testing. :)