pimatic / pimatic-homeduino

Pimatic plugin for using 433mhz devices and sensors with a connected Arduino with homeduino sketch
https://pimatic.org/
GNU General Public License v2.0
37 stars 29 forks source link

RfSwitch with multiple protocols #3

Closed Icesory closed 9 years ago

Icesory commented 9 years ago

Warning: The Config for RFSwitch was changed Edit: it is now a new device

sweetpi commented 9 years ago

Wow.

+1 for the the new protocols options, but I don't think that we should create a separate class for this. Multiple protocols are also useful for dimmer, shutters, buttons,... So we would have to duplicate all classes.

I would change the config of all existing classes and add a detection for "old style" configs and convert them automatically, so nobody's config breaks on update. Do you want to change it? I would then add the converting of old config options.

Pingback: pimatic/rfcontroljs#14

Icesory commented 9 years ago

Oh yes this is the best way. Ok i will try my self on the config detection. Its right to do this in the constructor?

sweetpi commented 9 years ago

Oh yes this is the best way. Ok i will try my self on the config detection. Its right to do this in the constructor?

No, because the config gets validated by the framework before it is passed to the constructor. So it would fail before we could correct it. But there is an additional parameter prepareConfig of registerDeviceClass that can be a function that does something with the config parameter, before it is validated.

https://github.com/pimatic/pimatic/blob/master/lib/devices.coffee#L689

Icesory commented 9 years ago

Öh. I solved the problem in the constructor. Maybe you can use this to adapt it to other Devices

Works fine with this three configs

    {
      "id": "old_and_new_switch",
      "name": "old_and_new_switch",
      "class": "HomeduinoRFSwitch",
      "protocol": "switch1",
      "protocolOptions": {
        "id": 9509718,
        "unit": 2
      },
      "protocols": [
        {
          "protocol": "switch1",
          "protocolOptions": {
            "id": 9509718,
            "unit": 0
          },
          "send": true,
          "receive": true
        },
        {
          "protocol": "switch1",
          "protocolOptions": {
            "id": 9509718,
            "unit": 1
          },
          "send": false,
          "receive": true
        }
      ]
    },
    {
      "id": "old_switch",
      "name": "old_switch",
      "class": "HomeduinoRFSwitch",
      "protocol": "switch1",
      "protocolOptions": {
        "id": 7654321,
        "unit": 0
      }
    },
    {
      "id": "new_switch",
      "name": "new_switch",
      "class": "HomeduinoRFSwitch",
      "protocols": [
        {
          "protocol": "switch1",
          "protocolOptions": {
            "id": 9509718,
            "unit": 0
          },
          "send": true,
          "receive": true
        },
        {
          "protocol": "switch1",
          "protocolOptions": {
            "id": 9509718,
            "unit": 1
          },
          "send": false,
          "receive": true
        }
      ]
    },
Icesory commented 9 years ago

Ich weiß es sollte englisch sein aber ich verstehe die tiefen des frameworks leider noch nicht so ganz. Wenn du mir das kurz erklären könntest wäre das echt genial. So wie ich das verstehe kann ich der prepareConfig eine funktion übergeben die dann ausgeführt wird. Nur wo muss diese funktion gespeichert sein.

Sorry for german. I asked for a deeper description of the framework and prepareConfig.

sweetpi commented 9 years ago

Sorry its easier to show it to you in action, then explaining it :) I will quickly add the changes and explain it.

sweetpi commented 9 years ago

This is the way it works: https://github.com/pimatic/pimatic-homeduino/commit/5e32d902f4f018dc8c816f84dd7b84c992359dfd#diff-4a6ab8ff9f43e024a64d1922ae68f41eR72

prepareConfig is a callback parameter for registerDeviceClass and is called with the config before validation. The supplied function the changes the old config to the new one. If you have questions feel free to ask.

Icesory commented 9 years ago

This is a huge improvement. Realy nice.

But why you use so many unless? xD For me it is harder readable as a if x is not y (i like this more x!=y) and a if p.receive is more readable as a unless p.receive is false https://github.com/pimatic/pimatic-homeduino/commit/5e32d902f4f018dc8c816f84dd7b84c992359dfd#diff-4a6ab8ff9f43e024a64d1922ae68f41eR197

(ist empfangen gleich richtig).dann (wenn nicht empfangen gleich falsch).dann

sweetpi commented 9 years ago

But why you use so many unless? xD For me it is harder readable as a if x is not y (i like this more x!=y) and a if p.receive is more readable as a unless p.receive is false

Yes its more difficult to read because its "doppelte verneinung". I used it because I was lazy :D I wasn't sure if the default values from the config schema were really added to the array elements. (I think they are...). Because the defaults are true I used

unless p.receive is false
  ...

So if p.receive is undefined then p.receive is false is false and the unless ... is true, so the protocol is used for receiving even if p.receive is undefined (what is the default).

With

if p.receive is true
  ...

a undefined p.receive would lead to false (what is not the default behavior).

Sorry for the confusion here. Will check later if the defaults are set to the config object and so if p.receive is true just works :)

Icesory commented 9 years ago

realy freaki in this condition. Thanks for the explanation. I readed now many lines of your code and there are many "unless". This complete new for me and i must think about this some seconds