How to optimize gas station prices processing (Tankerkoenig)

I am in the process of reworking my whole openHAB installation and startet with my gas station prices. I get the prices from the tankerkoenig binding and so far ist is working very well.

On the other hand it is working not in an optimized manner. If one the the gasstation prices changes, a rule is triggered that sorts the prices from 6 stations and displays the prices in a sorted order.

Due to the fact that the changes from all six station come very close together i created some latching logic. Unfortunately if all six station change their price the rule is triggered six times and all verlues are sorted six times.

My idea ist now to start the rule by the first trigger, then start a timer that delay the sorting by 5 minutes and ignore all subsequent triggers.

Unfortunately i have not idea at the moment how to do this.

This is the current rule

rule "Process Gas Station Prices"
when
    
    Member of gGasStationPrices changed

then
    
    val String ruleIdentifier = ruleIdentification+"ProcessGasStationPrices"

	try {

		gasLatch.lock()

		var counter = 1

		logInfo(ruleIdentifier, "Rule triggered by {}.", triggeringItem.name.toString)

		for (gasPrice : gGasStationPrices.members.sortBy[ state as DecimalType ]) {
		
		    val String gasKey = gasPrice.name.substring(1, gasPrice.name.lastIndexOf('_'))

			var ContactItem stationOpen = ScriptServiceUtil.getItemRegistry.getItem("i"+gasKey+"_Open") as ContactItem
			var StringItem stationDisplay = ScriptServiceUtil.getItemRegistry.getItem("iSL_OH_GasStationItems_Display"+String::format("%02d", counter)) as StringItem
			
			stationDisplay.setLabel(transform("MAP", "i18n_gasstation.map", gasPrice.name.substring(1, gasPrice.name.lastIndexOf('_'))))

			if (stationOpen.state == OPEN) {

				stationDisplay.postUpdate(String::format("%.3f €", (gasPrice.state as DecimalType).doubleValue()))

			} else {

				stationDisplay.postUpdate(transform("MAP", "i18n_contact.map", stationOpen.state.toString))

			}
			
			counter++
			logInfo(ruleIdentifier, "Price for station {} processed.", gasKey)

		}

	} catch(Exception e) {

		logError(ruleIdentifier, "Error in processing gas station prices: " + e.toString)
		gasLatch.unlock()
	
	} finally {

		gasLatch.unlock()

	}

end

Any ideas?

The first question to ask is whether this is actually a problem and if so is it worth solving?

I’d say it’s a fun problem to solve so its worth addressing for that reason. If you are worried about the efficiency of the Rule, it probably doesn’t make a difference so let it be.

That would be my approach.

First of all you don’t need the lock. If you use regular Timers then the first line of the Rule checks to see if there is a timer and if so immediately exit. If using Expire based Timers, check to see if the Timer Item is ON.

If there isn’t a timer, create/trigger one. In the body of the Timer/Rule that triggers when Expire sends command OFF you do the sorting. Make sure to set the timer back to null if using regular Timers.

Here is what it would look like with Expire based timers.

rule "Process Gas Station Prices"
when
    Member of gGasStationPrices changed
then
    // if Timer isn't ON, set the timer, otherwise we ignore this changed event, it will be handled later
    // when the Timer goes OFF
    if(GasStationPricesTimer.state != ON) GasStationPrices.sendCommand(ON)
end

rule "Time to sort the prices"
when
    Item GasStationPricesTimer received command OFF
then
    // Sort your prices
end

Looking at the sorting Rule, there are some opportunities to improve this code a little, mainly for readability.

gGasStationPrices.members.filter[price | price != UNDEF && price != NULL].sortBy[ state as DecimalType ].forEach[ gasPrice, index |
    val gasKey = gasPrice.name.substring(1, gasPrice.name.lastIndexOf('_'))
    val stationOpen = ScriptServiceUtil.getItemRegistry.getItem("i"+gasKey+"_Open") as ContactItem
    val stationDisplay = ScriptServiceUtil.getItemRegistry.getItem("iSL_OH_GasStationItems_Display"+String::format("%02d", index+1)) as StringItem

    if (stationOpen.state == OPEN) {
        stationDisplay.postUpdate(String::format("%.3f €", (gasPrice.state as DecimalType).doubleValue()))
    } else {
        stationDisplay.postUpdate(transform("MAP", "i18n_contact.map", stationOpen.state.toString))
    }
]
logInfo(ruleIdentifier, "Price for station {} processed.", gasKey)
1 Like

@rlkoshak Thank you for your feedback.

You are totally right. It is more a fun problem than a real problem. But it helps learning things.

More and more i come to the conclusion that i have to have a closer lot at the expire binding. Your code is very easy.

If i see it right you changed the loop. Introducing a second index (i like that idea).

Again Thanks a lot that helps a lot. I will pist my final solution.

I now try to build a version based on a timer, but from the log i see that it is not working proberly.

rule "Process Gas Station Prices"
when
    
    Member of gGasStationPrices received update

then
    
    val String ruleIdentifier = ruleIdentification+"ProcessGasStationPrices"

    if (delayTimer !== null) {
        logInfo(ruleIdentifier, "Gas station price sorting already triggered, ignoring additional request from {}.", triggeringItem.name.toString)
        return;
    }

    logInfo(ruleIdentifier, "Gas station price sorting is triggered by {}.", triggeringItem.name.toString)
    delayTimer = createTimer(now.plusSeconds(delayValue), [ |

        logInfo(ruleIdentifier, "Gas Station prices are sorted.")
        delayTimer = null

    ])
    
end

and this is the log

2019-07-13 13:16:17.687 [INFO ] [sstationrule.ProcessGasStationPrices] - Gas station price sorting is triggered by iSL_IN_GasStation06_Diesel.
2019-07-13 13:16:17.692 [INFO ] [sstationrule.ProcessGasStationPrices] - Gas station price sorting is triggered by iSL_IN_GasStation04_Diesel.
2019-07-13 13:16:17.699 [INFO ] [sstationrule.ProcessGasStationPrices] - Gas station price sorting is triggered by iSL_IN_GasStation03_Diesel.
2019-07-13 13:16:17.699 [INFO ] [sstationrule.ProcessGasStationPrices] - Gas station price sorting is triggered by iSL_IN_GasStation01_Diesel.
2019-07-13 13:16:17.702 [INFO ] [sstationrule.ProcessGasStationPrices] - Gas station price sorting is triggered by iSL_IN_GasStation05_Diesel.
2019-07-13 13:18:17.690 [INFO ] [sstationrule.ProcessGasStationPrices] - Gas Station prices are sorted.
2019-07-13 13:18:17.696 [INFO ] [sstationrule.ProcessGasStationPrices] - Gas Station prices are sorted.
2019-07-13 13:18:17.701 [INFO ] [sstationrule.ProcessGasStationPrices] - Gas Station prices are sorted.
2019-07-13 13:18:17.704 [INFO ] [sstationrule.ProcessGasStationPrices] - Gas Station prices are sorted.
2019-07-13 13:18:17.703 [INFO ] [sstationrule.ProcessGasStationPrices] - Gas Station prices are sorted.

So the check if a delayTimer is running is never executed.

Any ideas?

Is the delayTimer global?

Yes

Now i tried it with the expire binding approach and get similar results in the log.

rule "Trigger Gas Station Price Processing"
when
    
    Member of gGasStationPrices received update

then
    val String ruleIdentifier = ruleIdentification+"TriggerGasStationPriceProcessing"

    if (iSL_OH_GasStationItems_Timer.state != ON) {
        
        iSL_OH_GasStationItems_Timer.sendCommand(ON)
        logInfo(ruleIdentifier, "Gas station price sorting is triggered by {}. Delay started.", transform("MAP", "i18n_gasstation.map", triggeringItem.name.substring(1, triggeringItem.name.lastIndexOf('_'))))

    } else {

        logInfo(ruleIdentifier, "Gas station price sorting already triggered, ignoring additional request.", transform("MAP", "i18n_gasstation.map", triggeringItem.name.substring(1, triggeringItem.name.lastIndexOf('_'))))

    }

end
2019-07-13 19:03:58.142 [INFO ] [ule.TriggerGasStationPriceProcessing] - Gas station price sorting is triggered by HEM (Elmshorn). Delay started.
2019-07-13 19:03:58.143 [INFO ] [ule.TriggerGasStationPriceProcessing] - Gas station price sorting is triggered by Nordoel (Elmshorn). Delay started.
2019-07-13 19:03:58.145 [INFO ] [ule.TriggerGasStationPriceProcessing] - Gas station price sorting is triggered by Star (Elmshorn). Delay started.
2019-07-13 19:03:58.154 [INFO ] [ule.TriggerGasStationPriceProcessing] - Gas station price sorting already triggered, ignoring additional request.
2019-07-13 19:03:58.158 [INFO ] [ule.TriggerGasStationPriceProcessing] - Gas station price sorting already triggered, ignoring additional request.

Perhaps they are happening too close together and they all make it past the if statement before the first time is created.

I’d change the rule trigger to use changed instead of update. That will eliminate some of the triggers when there is no change.

Since they happen so close together (less then 10 msec) a lock may be the only option. Since that’s the case you have to ask whether the danger of using a lock, which has all sorts of ways things can go wrong, is worth the benefit of having a rule not run repeatedly unnecessarily, which isn’t causing any real problems.

did you initialise delayTimer to null?

var Timer delayTimer = null

@JimT Yes it is initialised to null.

@rlkoshak I think the events are to close to gether and the “problem” will increase if i use faster hardware. So i have to stick with my initial solution or build something new. The hint to use “changed” as a trigger is obvious. I used the “update” trigger only because is triggers regulary every 15 minutes.

Thanks for your ideas

I’m curious. Can you try this:

rule "Process Gas Station Prices"
when
    
    Member of gGasStationPrices received update

then
    
    val String ruleIdentifier = ruleIdentification+"ProcessGasStationPrices"

    if (delayTimer !== null) {
        logInfo(ruleIdentifier, "Gas station price sorting already triggered, ignoring additional request from {}.", triggeringItem.name.toString)
        return;
    }

    logInfo(ruleIdentifier, "Gas station price sorting is triggered by {}.", triggeringItem.name.toString)
    if (delayTimer === null) {
        delayTimer = 1 // not sure if this will work due to type issue, but presumably this should execute very quickly compared to createTimer()
        delayTimer = createTimer(now.plusSeconds(delayValue), [ |
            logInfo(ruleIdentifier, "Gas Station prices are sorted.")
            delayTimer = null
        ])
    }
end

Just realize that you are “fixing” something that isn’t a problem with something that has the potential to stop all of your rules from executing. For example, if one instance of the rule crashes in a way where the lock never gets unlocked (which can happen even with a try/catch/finally) all the other instances will be stuck waiting for the lock and keep any other rule from running.

That seems to be a pretty severe risk to solve a pretty benign annoyance.

Is there any way you can move this further up the chain? Maybe vary the poll times so only one it to changes at a time? Or you get all all the values in one message that you then parse in a single rule?

If not I’d say it’s safer and better to leave well enough alone.

If have gone back to my initial solution with your sorting optimisation. And it works well.

but maybe there is another solution without any risk. My initial approach was to display the gas sation prices in a sorted order so that i see the cheapest first. So i need to see the station name and the price.

The binding polls a webservice and updates the items after each poll (every 15 Minutes). Another way is ti poll the service on my own, but i think that is not the solution.

What about splitting the rule into two (based on the expire binding approach):
1.

rule "Timer Gas Station Price Processing"
when
    
    Member of gGasStationPrices changed

then
    postUpdate.iSL_OH_GasStationItems_Timer(ON)
end
rule "Timer Gas Station Price Processing"
when
    Item iSL_OH_GasStationItems_Timer changed to ON
then
    //sort gastation logic
end

Using the binding updates will come only at the scheduled times. I would use a crown triggered rule iot update the sorting. Look into the log at what time the poll by the binding is done and schedule the rule to trigger some seconds after.
Yes, that way the cron has to be reset each time the binding schedule is changed.

@Syn My first approach with two rules did not succeed, but i wil give it a try. The main difference i see is that you dont care about retriggering the expire timer. That might be the solution.

@opus Thats a solution, but i do not want to change the cron statement every time i change something.

Totally understand the reasoning!
What you WOULD need is a single event raised by the binding each time the prices are polled. Such would be possible (although, as of now,I do’t know how to code it), however that would add an additional event (besides the update/changed event of each price-channel) just for this purpose.

There is no way to ask the binding to use a different polling time for each station? Anything at all to stagger the updates to the Items so they are less likely to all get updates at the same time.

Polling the service yourself could be a solution.

Given how minor the issue is with the Rule running more often then necessary and how severe the potential problems that can be caused by using a lock, I cannot recommend that approach. The risks from the lock far outweigh the original problem.

If you want to fix this minor problem, the only way I see is to move back further in the chain so you can either try to keep the Items from all updating at the same time or get the raw data and update your sorted Items from that.

That could work. The Timer will get updated to ON a bunch of times but then X seconds after the last update it will sort the values. This is a good solution as long as the number of seconds the Expire is set to is shorter than the frequency that the Items are updated. You want to make sure that the sorting Rule is done before the next update or else you will be sitting there waiting forever and the Expire timer never goes off.

I like this approach.

I thought of the cron triggered approach too and was going to post that this morning. I think I like Christoph’s approach a little better but cron would be a good approach too.

That is the key difference. Instead of ignoring all the events after the first one, you ignore all the events until the last one. It’s a subtle difference that I think will make the approach viable.

Yes there is a way. Use a separate Webservice for each fuel station. However, such would increase the load on the Tankerkoenig server, which they asked not to do!
The approach posted by @Syn sounds best to me as well.
Since all updates do come from a single poll the sorting should be triggered after any poll that made a change. Since the poll is done in seconds an exipre time of about 10 seconds should be long enough.

@opus @rlkoshak

Tankerkoenig’s API offers two way to query a gas station. First is to query a single station and second is query upto 10 stations at once. Tankerkoenig request to use the second solution to keep the load of the server low.

I tried @ Syn solution now an with all the optimisations it is working now. Next step is to bring the display to Habpanel. I will post the items and rules later.

1 Like