Added Homesensor

[MGPhil]

Was a bit tricky but figured out how to add two ContactSensors and control them separately.

Code was tested on my local setup and both error sensor and new home sensor works as intended.

Would be great if you could accept the changes and publish this new version to npm than it is easily installable via homebridge ui.

Thanks

[normen]

Very nice, thanks. The only problem I see is that existing setups might get messed up because the HomeKit - stored accessory doesn't have a sensor named "ErrorSensor", meaning installing the update would require people to reload their mowers or run into trouble with the plugin.

Can you check this and possibly mitigate this - e.g. by checking if the "ErrorSensor" is actually returned and if not check for a single contact sensor?

[MGPhil]

I have just copied the new index.js file over my old install before the update (even without the other config files) and new sensor showed up and old error works as well without any reloads, may homebridge/homekit is intelligent enough to sort that out as its just an internal code link name. Externally the first defined contact sensor stays the first one with the same function (Error message). I think once I screwed something up the plugin made a reload itself but worked before as described as well.

If you still think I should implement an if check to use the old code with just one unnamed contact sensor and only use the named contact sensors if defined in the config then let me know.

Would be great if you can tell me the best way to check if a variable exists in the config file? Like If(this.config.homesensor.exsists == true) …. True/False is clear but check if defined/exists not, thanks.

[normen]

For the config value, you can compare to undefined in Javascript. When you use a triple equals (value === undefined) it will only check for undefined, otherwise in Javascript 0 == undefined == null == false is true, which can sometimes be confusing.

For the error sensor, if I get it right, you're only grabbing a Service named "ErrorSensor" right? Can't imagine that would work without issues.. I didn't check that yet though to be honest.. I guess a small check would be in order.

If the plugin fails to load then HomeKit would throw away that plugins settings in the cache, so yeah, homebridge might crash, forget about the plugin and next time grab the mowers again from the cloud but that neither seems to be a clean solution nor would it keep any automation associated with that HomeKit device..

[MGPhil]

Seams logic, ok then I will modify it with that view lines of code to have no potential issues for users with existing installs.

[normen]

You don't need to close this, simply pushing changes to the same branch will do.

[MGPhil]

Will reopen when Update is finished, good to know, some problem I run into while updating it:

function LandroidAccessory(platform, name, serial, accessory) {
    this.landroidCloud = platform.landroidCloud;
    this.log = platform.log;
    this.config = platform.config;

    if (accessory) {
      this.accessory = accessory;
    } else {
      // new accessory object
      var uuid = UUIDGen.generate(serial);

When I make an update like that in the LandroidAccessory Structure above in the else:

if(this.config.homesensor === undefined){
        this.accessory.addService(new Service.ContactSensor("Landroid " + name + " Problem"));
      } else{
        this.accessory.addService(new Service.ContactSensor("Landroid " + name + " Problem", "ErrorSensor"));
      }

Then that code gets only executed at users first run of the plugin and runs for example the if line. If I add then later the homesensor to the config (either false or true) the accessory is defined and that part is not rerun so the else "ErrorSensor" is never defined like that and therefore the script is not working / giving errors and crashing homebridge.

Any idea how to redefine the accessory if already there? If I clear all mowers then the script reruns the code part but that is exactly what we want not to happen...

[normen]

Yeah, thats normal behavior. The Accessory is stored in the HomeKit cache which the first branch of the if is for. Thats why theres the option to "reload all mowers", which is also needed when you activate e.g. the rain sensor. I guess you could removeService or something but then that should be added for all optional sensors and I'm not quite sure if it would count as the same device in HomeKit when the configuration changes, hence reloading the mowers (which basically just deletes the devices in Homekit and then re-creates them) is the current workaround.

You also don't need the last bit of code really, always calling the sensor "ErrorSensor" for new Accessories is fine, the code should just expect that theres configurations where that is not the case (i.e. the existing setups of users which are loaded from the HomeKit cache). Just a small if check in the code of the previous PR would be fine.

[MGPhil]

Ok so you means go back to the original pull request code and just add two if that check if „ErrorSensor“ is defined and if not use the old code?

In the accessory creator it can be always the new „structure“ as that one is only used if the accessory was never created or cannot be loaded, than it does not matter correct?

That just leads to the question how to check if a contact sensor exists that is named „ErrorSensor“ and how to best go back to and old code version (Copy&Paste complete code from github history as new commit maybe)?

[normen]

I guess you can just separate the getService and see if it returns null or something? You could try what happens by asking for a service that definitely doesn't exist. e.g. console.log(this.accessory.getService("Cthulhu"));

Then just make that something like:

var myService = ...getService("ErrorSensor")
if(!myService) myService = ...getService(Service.ContactSensor)
myService.getCharacteristic......

Otherwise yes, when the second branch of the if-check is executed we're creating a new Accessory in HomeKit anyway, doesn't matter how we call it then. The only issue is that the current code would blow up peoples existing setups when they update the plugin. Its just for backwards-compatibility.

[normen]

Actually we're already using if(this.accessory.getService("HomeSensor")) so the above pseudo-code should work, I guess it does return null if it doesn't exist.

[normen]

Oh and for how to get back, you can just revert your local changes, afaict you didn't commit any of those to your master branch?

[MGPhil]

Will try to find time implementing an testing in the next days. Code change is not the problem but I would like to test all features with my local setup before the final pull request ;-)

As I have done all updates in the browser with edit and commit I think the only way is viewing in the history the old version, copy all code from there and paste it over the current version as new commit? Or any other Ideas? Could also change the lines back manually but slower and should not make a difference or?

[normen]

Right, the changes don't appear here because you closed the PR, otherwise we'd see the changes here. Yeah, I guess that would be the easiest way then, just open a past commit and copy-paste.

[MGPhil]

Tested everything and works like expacted without any impact when updating from existing setup:

• Installed original index.js file, installed in my home environment and moved in my room
• Updated to new index.js and leave config as it was => works
• Changed config (added "homesensor": false) => works
• Changed config (changed "homesensor": true) => works but home sensor not showing up as accessory loaded and not generated new
• Changed config (added "reload": true) => mower moved back to "standard" room and home sensor shows up => everything works
• Changes config (removed "reload": true) => works like expected and stays in moved room

[normen]

Cool, thanks, this looks good. I'll merge and publish in the coming days. Remind me if life meddles with that plan 😄