Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Schedule no longer works #1022

Closed
ebaauw opened this issue Dec 10, 2018 · 17 comments
Closed

Schedule no longer works #1022

ebaauw opened this issue Dec 10, 2018 · 17 comments

Comments

@ebaauw
Copy link
Collaborator

ebaauw commented Dec 10, 2018

I overslept this morning, since my wakeup schedule didn't fire. Upon inspection, it's action contains a username (api key) in the address that I deleted months, if not years, ago. I suspect the changes in the API handling (9e9d092) now block the action.

@manup
Copy link
Member

manup commented Dec 10, 2018

Yes looks like auth handling needs more careful testing, had similar issue today.

ffca4f9

@manup
Copy link
Member

manup commented Dec 10, 2018

Can you please provide the schedule content, looking at the code it should still work since the auth checks are not done at this level.

@KodeCR
Copy link
Contributor

KodeCR commented Dec 10, 2018

Oops, sorry... before the changes the schedules node wasn’t checked for auth at all. That was one of the reasons to move it to the top level, so it not so easy to forget one. So basically this fixed a bug. If you change your rule to use an authorised api key it should work again. ?

@ebaauw
Copy link
Collaborator Author

ebaauw commented Dec 10, 2018

I think so. Will see tomorrow morning...

@ebaauw
Copy link
Collaborator Author

ebaauw commented Dec 11, 2018

No joy, schedule didn't fire this morning either. Good thing that I did set a backup alarm...

Here's the schedule (with the updated API key):

$ ph get /schedules/2
{
  "activation": "start",
  "autodelete": false,
  "command": {
    "address": "/api/85E340F30E/sensors/321/state",
    "body": {
      "status": -2
    },
    "method": "PUT"
  },
  "description": "Wakeup Alarm Week",
  "etag": "d23a2e9c8bd30ec9ce33c9d5600ecfbf",
  "localtime": "W120/T07:00:00",
  "name": "Wakeup Alarm Week",
  "status": "enabled",
  "time": "W120/T07:00:00"
}
$ ph get /config/whitelist/85E340F30E
{
  "create date": "2018-01-27T16:00:49",
  "last use date": "2018-12-10T17:51:46",
  "name": "ph#pi2"
}
$ ph get /sensors/321
{
  "config": {
    "on": true,
    "reachable": true
  },
  "etag": "4b699b9f694c8add98d7f013d4b5725c",
  "manufacturername": "homebridge-hue",
  "modelid": "CLIPGenericStatus",
  "name": "Bedroom Status",
  "state": {
    "lastupdated": "2018-12-11T07:41:28",
    "status": 1
  },
  "swversion": "-2,3",
  "type": "CLIPGenericStatus",
  "uniqueid": "/sensors/321"
}

@manup
Copy link
Member

manup commented Dec 11, 2018

By checking the code it should work even with non existing/invalid apikey.
The schedules are evaluated within the schedule timer where unlike for external http requests the authentication isn't checked.

https://github.com/dresden-elektronik/deconz-rest-plugin/blob/master/rest_schedules.cpp#L1034

@ebaauw
Copy link
Collaborator Author

ebaauw commented Dec 11, 2018

Did some more digging in the logs. I'm seeing the same messages for the schedule:

Dec 11 07:00:00 pi2 deCONZ[30157]: 07:00:00:779 schedule 2: Wakeup Alarm Week trigger
Dec 11 07:00:00 pi2 deCONZ[30157]: 07:00:00:779 schedule 2 body: {"status":-2}

But I no longer see the web socket notification, nor a message for the rule that should be triggered.

Dec  6 07:00:00 pi2 dc_eventlog[6411]: /sensors/321/state: {"lastupdated":"2018-12-06T06:00:00","status":-2}
...
Dec  6 07:00:02 pi2 deCONZ[28717]: 07:00:00:285 schedule 2: Wakeup Alarm Week trigger 
Dec  6 07:00:02 pi2 deCONZ[28717]: 07:00:00:285 schedule 2 body: {"status":-2}
Dec  6 07:00:02 pi2 deCONZ[28717]: 07:00:00:288 rule event: /sensors/321 state/status num (-1 -> -2)
Dec  6 07:00:02 pi2 deCONZ[28717]: 07:00:00:288 trigger rule 197 - Bedroom Wakeup 1/3

As my backup alarm, I set the status from HomeKit - that did issue the web socket notification and did trigger the rule alright.

So it would seem that a schedule command no longer triggers the event?

@manup
Copy link
Member

manup commented Dec 11, 2018

Dec 6 07:00:02 pi2 deCONZ[28717]: 07:00:00:288 rule event: /sensors/321 state/status num (-1 -> -2)
Dec 6 07:00:02 pi2 deCONZ[28717]: 07:00:00:288 trigger rule 197 - Bedroom Wakeup 1/3

I'm a bit confused this looks like the rule is triggered based on the event?

@ebaauw
Copy link
Collaborator Author

ebaauw commented Dec 11, 2018

Yeah, that's the old log (Dec 6) from 2.05.49. These messages are missing from the 2.05.50, Dec 11 log.

@manup
Copy link
Member

manup commented Dec 11, 2018

Ah ok sorry missed that :) I'll try to recreate similar setup and see where it hangs.

@ebaauw
Copy link
Collaborator Author

ebaauw commented Dec 11, 2018

schedule failed: 404 Not Found [{"error":{"address":"/lights/3","description":"resource, /lights/3, not available","type":3}}]

It tries to handle the /sensors command as a /lights request,

@ebaauw
Copy link
Collaborator Author

ebaauw commented Dec 11, 2018

And that's because the check for the right request type has been removed from handleLightsApi() (and from the other handlers).

    if (req.path[2] != QLatin1String("lights"))
    {
        return REQ_NOT_HANDLED;
    }

Oh, and indeed, the api key isn't checked ;-)

@manup
Copy link
Member

manup commented Dec 11, 2018

Unexpected behavior ;)
Ok I think simplest fix will be bringing back the checks. The schedule timer could also dispatch but I'm worried there might be other places with similar handling.

@ebaauw
Copy link
Collaborator Author

ebaauw commented Dec 11, 2018

but I'm worried there might be other places with similar handling.

Indeed, especially since the check wasn’t just with lights, groups and sensors, but also with e.g. resourcelinks. I could create a PR, but that might have to wait until this weekend.

@KodeCR
Copy link
Contributor

KodeCR commented Dec 12, 2018

I removed the check because it 's also done at top level. Is handleLightsApi called directly when the schedule triggers? If so, factoring out the top level API handling and calling that from both places would be better?

@ebaauw
Copy link
Collaborator Author

ebaauw commented Dec 12, 2018

Then we’d lose the check that the command should only be for lights, groups, or sensors. Also, this would introduce checking the username/api key.

manup added a commit that referenced this issue Dec 12, 2018
@ebaauw
Copy link
Collaborator Author

ebaauw commented Dec 18, 2018

Fixed in v2.05.52.

@ebaauw ebaauw closed this as completed Dec 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants