maxwellhadley / node-red-contrib-rfxcom

node-RED nodes to access an RFXtrx433 transceiver
BSD 2-Clause "Simplified" License
22 stars 13 forks source link

X10 Motion not reset #64

Closed bertrand1 closed 5 years ago

bertrand1 commented 5 years ago

Hi I received PIR motion detector : the model is Kerui P831 Connected with RFXcom EXT2 V1025, I can see each time a event is triggered ,a packet is catched and ready to be process by rfx-detector node

So the first time an event is triggered, rfx-detector decode frame and capture data[7] -> 04 ->Motion In the case Payload :"Motion" is provided Good

But Kerui P831 only sends Motion events and is not able to send Normal state.

Because in rfxcom.js file at line 969 you only manage motion event if laststatus has changed, the result is for this kind of sensors which only send motion ( and not Normal or No Motion ) :

May be you can add a check box in the rfx-detector node to force process event even if the last status was the same.

Tell me if I can help

Thanks Bertrand

maxwellhadley commented 5 years ago

Does the Kerui P831 send the 'alive' heart-beat messages like the other X-10 sensors? If not, you should get a 'Silent' message in Node-RED 90 minutes after the last 'Motion' message. Can you check this, please?

If there is no way to distinguish between a 'genuine' X10 PIR and the Kerui, I will have to re-think this logic to cope with both!

bertrand1 commented 5 years ago

Hi Max

I did the test As the Sensor does not send keepalive or heartbeat message , so I got a Silent message after 90 minutes

What would be your solution to have this kind of PIR/motion working with your library ?

Again tell me if I can help .

Thanks

Bertrand

2 Dec 14:25:11 - [info] Starting flows
2 Dec 14:25:11 - [info] Started flows
2 Dec 14:25:11 - [info] [mqtt-broker:mqtt local] Connected to broker: mqtt://localhost:1883
2 Dec 14:25:11 - [info] [rfx-detector-in:2e0604c8.04dc1c] connecting to /dev/ttyUSB3
2 Dec 14:25:17 - [rfxcom] on /dev/ttyUSB3 - Sent    : 0D,00,00,00,00,00,00,00,00,00,00,00,00,00
2 Dec 14:25:17 - [rfxcom] on /dev/ttyUSB3 - Sent    : 0D,00,00,01,02,00,00,00,00,00,00,00,00,00
2 Dec 14:25:17 - [rfxcom] on /dev/ttyUSB3 - Received: 14,01,00,01,02,53,19,0A,00,27,00,01,03,1C,04,52,46,58,43,4F,4D
2 Dec 14:25:17 - [rfxcom] on /dev/ttyUSB3 - Sent    : 0D,00,00,02,07,00,00,00,00,00,00,00,00,00
2 Dec 14:25:17 - [rfxcom] on /dev/ttyUSB3 - Received: 14,01,07,02,07,43,6F,70,79,72,69,67,68,74,20,52,46,58,43,4F,4D
2 Dec 14:25:17 - [rfxcom] on /dev/ttyUSB3 - Copyright RFXCOM
2 Dec 14:25:17 - [rfxcom] on /dev/ttyUSB3 - Started command message queue
2 Dec 14:25:17 - [info] [rfx-detector-in:2e0604c8.04dc1c] connected: Serial port /dev/ttyUSB3

2 Dec 14:25:51 - [rfxcom] on /dev/ttyUSB3 - Received: 08,20,01,03,88,15,00,04,89
2 Dec 14:25:51 - [info] [debug:Detector] 
{ topic: 'X10_PIR/0x881500',
  status: { rssi: 8, battery: 9, tampered: false, state: 4, delayed: false },
  payload: '**Motion**',
  _msgid: 'd8b8fb02.abd5c8' }

2 Dec 15:53:15 - [info] [debug:Detector] 
{ topic: 'X10_PIR/0x881500',
  payload: 'Silent',
  lastMessageStatus: { rssi: 8, battery: 9, tampered: false, state: 4, delayed: false },
  lastMessageTimestamp: 1543756994099,
  lastHeardFrom: 'Sun, 02 Dec 2018 13:23:14 GMT',
  _msgid: '3558f315.de1b6c' }

2 Dec 15:55:52 - [info] [debug:Detector] 
{ topic: 'X10_PIR/0x881500',
  payload: 'Silent',
  lastMessageStatus: { rssi: 8, battery: 9, tampered: false, state: 4, delayed: false },
  lastMessageTimestamp: 1543757151774,
  lastHeardFrom: 'Sun, 02 Dec 2018 13:25:51 GMT',
  _msgid: '59ae5e09.6ac54' }

2 Dec 16:10:13 - [rfxcom] on /dev/ttyUSB3 - Received: 08,20,01,21,88,15,00,04,89
2 Dec 16:10:21 - [rfxcom] on /dev/ttyUSB3 - Received: 08,20,01,22,88,15,00,04,89
2 Dec 16:10:41 - [rfxcom] on /dev/ttyUSB3 - Received: 08,20,01,23,88,15,00,04,89
2 Dec 16:10:54 - [rfxcom] on /dev/ttyUSB3 - Received: 08,20,01,26,88,15,00,04,89
2 Dec 16:11:07 - [rfxcom] on /dev/ttyUSB3 - Received: 08,20,01,27,88,15,00,04,89
maxwellhadley commented 5 years ago

OK I think a review of the logic behind generating 'silent' messages is needed. I will think about the best way to do this over the next few days. I also see that doing a partial deployment (which restarts the flow, but does not affect the rfx-detector node) does not clear the heartbeat timer - which is why you get two 'silent' messages less than 3 minutes apart.

bertrand1 commented 5 years ago

Hi Max

Understood.

Will wait for your release. If you need a beta tester, don't hesitate .

Thanks Bertrand

maxwellhadley commented 5 years ago

I have just pushed branch handle-zombie-x10-PIRs to GitHub. This copes automatically with devices like the Kerui, which are alive, but don't have a heartbeat (clearly zombies!). The heartbeat timer mechanism will now only be enabled for a device if that device sends at least one 'normal' status message. I have also changed the logic so that repeated 'motion' messages from a sensor will now each generate an output Node-RED message. This is required to support the Kerui detectors, but actually it also makes sense for proper X10 PIRs, as there may well be multiple, legitimate motion detections before the heartbeat message is due to be sent.

There is no change to the UI, and it should be fully compatible existing flows - please try it out!

bertrand1 commented 5 years ago

Hi Max I've tested your fix but there is still an issue

In fact the condition if (deviceIsNotInAlarm || isNaN(lastDeviceStatus) === false) is never true in my case because the only message that initializes the lastDevicesStatus variable should done when a motion event is received Still in my case your fix works with this condition :

                    if (evt.deviceStatus === rfxcom.security.MOTION) {
                        msg.payload = "Motion";
                        if (isNaN(lastDeviceStatus)){
                            node.heartbeats[msg.topic] = {
                                lastStatus: evt.deviceStatus,
                                interval:   (function () {
                                    const heartbeatStoppedMsg = {
                                        topic:                msg.topic,
                                        payload:              "Silent",
                                        lastMessageStatus:    msg.status,
                                        lastMessageTimestamp: Date.now(),
                                        lastHeardFrom:        new Date().toUTCString()
                                    };
                                    return setInterval(function () {
                                        delete heartbeatStoppedMsg._msgid;
                                        node.send(heartbeatStoppedMsg);
                                    },  60*1000*node.HEARTBEATDELAY[evt.subtype]);
                                }())
                            };
                            // This ensures a clean shutdown on redeploy
                            node.heartbeats[msg.topic].interval.unref();

                        }
                    }

Thanks Bertrand

maxwellhadley commented 5 years ago

I don't see your problem here. I agree that(deviceIsNotInAlarm || isNaN(lastDeviceStatus) === false) will never be true with a Kerui sensor, since they only ever send events with status = security.MOTION. But this condition guards only setting the heartbeat timer: a normal Node-RED message with payload = "Motion" is sent by the node whether or not a timer is set, and it is sent regardless of lastEventStatus - that variable now affects only Alarm & Normal status messages:

// Payload priority is Tamper > Alarm/Motion/Normal > Battery Low
if (evt.batteryLevel === 0) {
    msg.payload = "Battery Low";
}
if (evt.deviceStatus === rfxcom.security.MOTION) {
    msg.payload = "Motion";
} else if (evt.deviceStatus !== lastDeviceStatus) {
    // Only report ALARM/NORMAL status (i.e. window open/closed) if the status has changed
    if (evt.deviceStatus === rfxcom.security.ALARM ||
        evt.deviceStatus === rfxcom.security.ALARM_DELAYED) {
        msg.payload = "Alarm";
    } else if (evt.deviceStatus === rfxcom.security.NORMAL ||
        evt.deviceStatus === rfxcom.security.NORMAL_DELAYED) {
        msg.payload = "Normal";
    }
}
if (event.tampered) {
    msg.payload = "Tamper";
}

You should now get a "Motion" message for each & every motion detection from your sensor, and no "Silent" messages. You definitely do not want to set a heartbeat timer!

Max

bertrand1 commented 5 years ago

Hi Max

Ok for me, only Motion received now with my Kerui P831. I manage the normal state directly in node-red.

Will you merge this patch in your master branch ?

Thanks a lot. Bertrand

maxwellhadley commented 5 years ago

Should now work OK