[SOLVED] Timer error message (another one :-) ): Could not invoke method: [….] on instance: null

[rlkoshak (Rich Koshak)]

@wawa79

What do you mean by complicated?

It is long for what it does. It creates variables that don’t need to be created (e.g. now_date). The constants for the amount of time to schedule the timers should be global, should be vals, and should not be ints. The conditions for the switch statement are not intuitive. What’s with the array lists?

I’m sure it all makes perfect sense to you, and that is what is most important, but for someone coming at it with no prior knowledge it’s really hard to figure out how it works and what it all means.

@wawa79

Do you refer to the repetition of statements in all case blocks that could be factored or is there anything else that would improve robustness of the code to avoid unexpected errors?

Not so much, though that can easily be handled by applying Design Pattern: How to Structure a Rule.

The complicated part is what the heck does DW_mCR_portail get set to? What about DW_mCR_portail? What does “CLOSED > PIETON” mean?

@wawa79

Would it make sense to add a lock() to the rule?

Please avoid locks unless absolutely necessary. They result in brittle code and run the risk of running out of Rules runtime threads.

@wawa79

Conclusion: next time I will post the entire rule and not an excerpt so you can blame me for being so stupid

Not stupid. We all make mistakes like this. I do believe though that VSCode would have identified this as an error since the problem is one of type.

Though having the full Rule often will give us the context necessary to help solve the problem.

---

So, I mentioned a number of problems I had with understanding your code. First let me emphasize that these are not problems with the code itself. But they are barriors to understanding the Rule for other developers who, like me, have not spent hours working on it.

Some recommendations I have that will make the code a little more self explanatory.

• Use now instead of creating new DateTime objects
• Always use val for constants. The standard naming convention is to use all caps for constants.
• Use Strings instead of numbers for what gets saved to DW_mCR_portail and new_mode. if(DW_mCR_portail.state == "OPEN") is much more clear to the person than if(DW_mCR_portail.state == 100)
• Avoid repeating code. Not only is it easier to read, it is more maintainable in the long run.

So the Rule could look something like:

val PORTAIL_OPENTIME = 26 * 1000 + 500
val PORTAIL_CLOSETIME = 25 * 1000 + 500
val PORTAIL_PIETONTIME = 6 * 1000 + 500

var Timer portail_task = null
var moveEnd = now

rule "Commande mCR_portail"
when
    Item DW_mCR_portail received command
then

    // 1. Decide if we need to run
    // Always run

    // 2. Calculate what needs to be done

    // calculate the new mode 
    var new_mode = ""
    switch(receivedCommand.toString) {
        case "CLOSED",
        case "CLOSING": new_mode = "OPEN"
        case "OPEN",
        case "PIETON",
        case "OPENING": new_mode = "CLOSED"
        default: new_mode = receivedCommand.toString
    }

    // Calculate the move end and new gate state
    val transition = receivedCommand.toString + " to " + new_mode // Never use the state of an Item inside a Rule where the Item receiving a command is the trigger of the Rule
    var currState = ""
    var portail_move_time = 0

    switch (transition) {
        case "CLOSED to PIETON": {
            portail_move_time = PORTAIL_PIETONTIME
            currState = "OPENING"
        }
        case "CLOSED to OPEN": {
            portail_move_time = PORTAIL_OPENTIME
            currState = "OPENING"
        }
        case "OPEN to CLOSED": {
            portail_move_time = PORTAIL_CLOSETIME
            currState = "CLOSING"
        }
        case "PIETON to CLOSED": {
            portail_move_time = PORTAIL_PIETONTIME
            currState = "CLOSING"
        }
        case "OPENING to CLOSED": {
            portail_move_time = moveEnd
            currState = "CLOSED"
        }
        case "CLOSING to OPEN": {
            portail_move_time = moveEnd
            currState = "OPENING"
        }
    }

    // 3. Do it
    // If we are OPENING or CLOSING stop the portail and start it again
    if(receivedCommand.toString.endsWith("ING")) {
        AC_mCR_portail.sendCommand(ON)
        createTimer(now.plusSeconds(1), [ | AC_mCR_portail.sendCommand(ON) ])
    }

    // Set the timer
    portail_task?.cancel
    portail_task = createTimer(now.plusMillis(portail_move_time, [ | 
            if(new_mode == "PIETON") AC_mCR_portail.sendCommand(ON)
            DW_mCR_portail.postUpdate(new_mode)
            portail_task = null
         ])
    moveEnd = portail_move_time
end

I’ve no idea if all of the above is correct but it should do the same thing as your original Rule. I can’t speak to whether there were not bugs in the original Rule. It is unclear whether the triggering Item will received the ING commands.