Enhancement: Variable autotimer

[Tom-Bom-badil]

For the time being, autotimer on an item is defined as follows: autotimer = time = value. Both time and value are fixed, no change possible during runtime.

I would like to recommend an amendment to item.py, so both can be read from another item (e.g. visu slider connector item) instead. The following amendment should work; however, there are possibly other ways how to solve this more elegant:

Old / as is:

# lib/item.py, starting line 491
def timer(self, time, value, auto=False):
	try:
		if isinstance(time, str):
			time = time.strip()
			if time.endswith('m'):
				time = int(time.strip('m')) * 60
			else:
				time = int(time)
		if isinstance(value, str):
			value = value.strip()

New / recommended changes (add 4 lines in order to identify item references and get the corresponding value):

# lib/item.py, starting line 491
def timer(self, time, value, auto=False):
	try:
		if isinstance(time, str):
			time = time.strip()
			if time.endswith('()')            # must be an item
				time = eval(time).strip() # get it's value
			if time.endswith('m'):
				time = int(time.strip('m')) * 60
			else:
				time = int(time)
		if isinstance(value, str):
			value = value.strip()
			if value.endswith('()')               # must be an item
				value = eval(value).strip()   # get it's value

I am particularly interested in a flexible time, but changing value might be of use for other folks, too. Thats why I put both here.

Thanks for all your work, and for considering this!

[ohinckel]

Changing the items timer()method seems to be the wrong location to add this feature. Usually this method is called at other places in Python code where the values for time and the actual value can be retrieved from other items directly.

I would change the way the autotimer is configured the item code. So item._autotimer could be a dict containing more information about the autotimer configuration (e.g. if the expressions needs to be evaluated or not). This should then be checked before the items timer() mehtod is called and - if configured - evaluate the expressions for time and value and pass the result to the timer() mehtod.

Maybe it would also be a good idea to introduce a new setting (like autotimer_eval) which can be used to configure this new behaviour.

autotimer = 10m = 0                   # old behaviour
autotimer_eval = sh.item() = sh.item() # new behaviour

Splitting the value and time configuration into two settings could also be helpful to make clear what is the time expression and what the value expression.

[Tom-Bom-badil]

100% agree. My focus was to find a quick solution that is downwards compatible and doesn't require a complete code rewrite (which will probably happen one day, considering the ongoing discussions about autotimer).

If I read the code right, a current call autotimer = sh.item() = sh.item() would trigger an error, so the suggested way shouldn't impact the mentioned 'other places' at all. I built a quick&dirty sh/sV test configuration and found the beforementioned error in my log, as expected.

Only conflict I can see at the moment is the definition of autotimer as quoted in your comment - these places would probably need to have the same evalutations, too - so 4 more lines of code by using copy/paste.

As said above - my approach for solving this was following the principle of least effort. Professional programmers will find for sure better and more elegant ways to do it.

Amendment: Putting it into timer() was on purpose, so the evaluation takes place in the moment the timer is called, not when the item is defined. This ensures that the time/value can be changed during runtime, too.

[wvhn]

we should merge this with #160. Same idea but without solution draft - which Tom has now proposed here.

[msinn]

#160 is not only about autotimer. I think we should sort it out. In #160 @ohinckel laid out some ways to handle eval in another way. Alternatively he proposed to fix it only for autotimer.

I think fixing the casting issue only for autotimer ist fine for the moment. The other cases could folllow later on.

If no one opposes, I would start implementing for autotimer:

• fixing that '5m' is allowed, '5s' isn't
• implementing the use of items in autotimer (incl. relative item addressing)
• fixing the casting issue

By fixing the casting issue we could introduce some incompatibilities. So my question: Should the implementation be done in autotimer or should I introduce something like autotimer_eval (as ohinkel proposed)?

Instead of implementing an autotimer_eval we could introduce a third parameter to tell shng to use the old or the new beaviour:

    autotimer = 5m = 42 = newcasting

The third parameter could be:

• newcasting - behave in the new fashion
• oldcasting - behave as the old sh and shng did
• empty - behave in the shNG default way. At the moment default way is the old way, but can be switched in an upcoming release.

We could use better names vor 'newcasting' and 'oldcasting'

Is it necessary to be able to specify a time like '5m30s' ?

[Tom-Bom-badil]

Either way (autotimer_eval or new-/oldcasting) is fine for me, as long as it will be possible to set the autotimer time/value by an item reference, which is evaluated in the moment before the autotimer is casted.

In my personal opinion it won't be necessary to set '5m30s' as this corresponds to a value of 330. Most time values are handled as seconds anyways, so the only advantage I can see in '5m30s' is a minor one - slightly better readybility. Taking into account that it will probably take some significant programming and testing, I would recommend to put your time into other/more important features. (just my 2 cents)

Just curious - what led you to the decision to not do it the 'easy way' by adding eval()'s as recommended above? It doesn't interfere, is downwards compatible and takes only a couple of minutes to implement. Is there something I didn't consider/see?

Anyways Martin, thank you a lot for your time and efforts!!! :)

[msinn]

I wanted it done for the next release. And going the eval() way look like beeing the way with far more decissions and compatibility issues (look at ohinkels comment in issue #160.

I am going to put in the item.__update routine to avoid interference with other calls to timer(). That way, the evaluation takes place every time the timer is set. The other part, the handling for relative item references has to be done someplace else. The conversion from relative to absolute references is done at the time the config is loaded.

[Tom-Bom-badil]

Thanks for all the work and the pull request - we just came across an issue that might be connected to this, could you have a look into this thread? If you feel it's not relevant, just disregard it. Thanks! :)

[walldi]

How about just evaluating the eval = .... Attribute when telling him to do so by setting eval_trigger = autotimer or eval_trigger = timer or eval_trigger = cycle?
I haven't tried ...

[msinn]

Won't work

[msinn]

Implemented for Version 1.3 and documented in wiki.