openhab / openhab-js

openHAB JavaScript Library for JavaScript Scripting Automation
https://www.openhab.org/addons/automation/jsscripting/
Eclipse Public License 2.0
38 stars 31 forks source link

[rules] Add event information for time-based and manual/rule-run triggers #286

Closed florian-h05 closed 10 months ago

florian-h05 commented 11 months ago

Refs https://github.com/openhab/openhab-core/pull/2965, therefore requires openHAB 4.0 Milestone 4 or newer.

digitaldan commented 11 months ago

Hi @florian-h05 sorry about being MIA the last few weeks. Work and life got really busy for me, hoping things calm down at the end of the month and i can start dedicating time to openHAB again. In the mean time, let me read up on the core change and I will give a review shortly.

florian-h05 commented 10 months ago

@digitaldan Have you had the time to test this?

digitaldan commented 10 months ago

Hmm, i'm not sure i am actually using this version of openhab.js, hard to tell from my logs (prob need to enable debugging). I have the binding set to "Do Not Use Included Library" and have this branch deployed to node_modules in my automation folder, but i am getting the following output for this test rule:

rules.when().cron("1 * * * * ?").then(e => {
    console.log(e);
}).build("CRON TEST");
{
  "eventClass": "org.openhab.core.automation.events.TimerEvent",
  "payload": {
    "GenericCronTrigger": "1 * * * * ?"
  },
  "module": "9d41475f-68cd-4e26-99ad-d130e4db770b"
}

Looking through the changes, I would expect to have a TriggerType and CronExpression member on the main event object ? I might poke at it some more today unless you have another idea?

florian-h05 commented 10 months ago

Hmm, when I tested this it worked fine.

I have uses JSRule and manually imported with require

Have you tried restarting the add-on or openHAB in general?

digitaldan commented 10 months ago

Yeah, i think the code is fine, my system refuses to not use the shipped js library, even after a restart. Sounds like a different issue. I would say merge this as i'm not going to be able to test until i figure out this other issue.

image