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

Autotimer: clarification and feature request #160

Closed
wvhn opened this issue Nov 23, 2016 · 9 comments
Closed

Autotimer: clarification and feature request #160

wvhn opened this issue Nov 23, 2016 · 9 comments
Assignees

Comments

@wvhn
Copy link
Contributor

wvhn commented Nov 23, 2016

In some cases the attribute
autotimer = 5m = 0
is not working correctly. There are discussions in the forum that the values behind autotimer are casted as string, e.g. here .
We have tested other applications where the attribute mentioned above works as expected.
Request:
the feature should be checked and/or documentation should be improved to clarify the influence of type specifications for items with autotimer - see here .

A new feature request is to give autotimer the ability to evaluate time and value parameters from items, e.g.
autotimer = sh.item1.time() = sh.item1.level()
This would add great flexibility to timers and make them controllable via visu.

Thanks Wolfram

@thehik
Copy link

thehik commented Nov 24, 2016

When eval of an item is called by autotimer, then

  • eval = value works as expected
  • eval = value and True gives always True

However, if value is a string, then the first example should always give True.

@ohinckel
Copy link
Member

We should add value = self.cast(value) before line to convert the value to the item's type. This will fix the problems with eval (and the autotimer issue with eval).

But poeple seem to use a value which is actually not allowed for the triggered item and use the value from the source item (which could be a string item) to update the destination item (which could be a bool item) by (explicitly) "converting" the value manually or (implicitly) automatically by Python in the eval expression. So changing this could lead to problem with these users since the convert may be different with this patch.

Nontheless, I would cast the value to the target item's type as mentioned above. Usually this explicit conversation should work like the implicit conversation. Only users using completely different values which can not be converted (e.g. a dict item to bool item) or doing some manually converting (e.g. extract information from a string value and use this for the target value) will have really problems.

So:

  • always convert and make it impossible to use a user-defined conversion in eval expressions
  • keep the current implementation and create a new eval_boxed setting (or another name like eval_casted, eval_converted, ...) which will get the value as casted value to item's type
  • do nothing at all and update the documentation, that the value will always be just a copy of the source value (is it a string item it will be a string, is it a bool item it will be a bool, is it from autotime it will be a string too)

To only fix the issue in relation to autotimer setting (which will currently use string values) we could add a cast when reading the autotime value and throw exception or a log message in case the conversion failed. But this make it impossible to use other values like text strings (containing some more meta infromation) for bool or num items.

To clarify, some examples:

[atint]
type = int
autotimer = 20m = 0  # will work with the current implementation

[atinteval]
type = int
autotimer = 20m = 0
eval = value  # will currently not work since value is string here, with explicit cast it would be a int and it will work then

[atintevalmeta]
type = int
autotimer = 20m = meta-0
eval = value.split('-')[1]  # will currently work but not with explicit cast, since the autotimer string value can not be casted to int

[evalitem]
type = num
eval = value  # will currently work since the cast is done when updating item, will also work with explicit cast
eval_trigger = itembool
[evalitem2]
type = num
eval = 1 if value is True else 0  # will currently not work, but will work with explicit cast
eval_trigger = itembool
[itembool]
type = bool

@wvhn
Copy link
Contributor Author

wvhn commented Nov 26, 2016

It is obvious that the value autotimer gives back to the calling item needs to be of the same type as the item is. A conversion function which does this automatically would give a great benefit and simplify usage for everybody.
Since I am not the expert to judge whether this is feasible or not, I could imagine to use a variable instead of a fixed value as workaround. This would be an outcome of the additional feature request, as well.

Example:

    [item]
       type = num
       autotimer = item.timer = item.value
       [[timer]]
           type = num
           eval ...
      [[value]]
           type = num
          eval ...

It should be very clear for any kind of user that item and item.value need to be of the same type then.

@onkelandy
Copy link
Member

Being able to use items for the autotimer would be awesome indeed.

By the way - there is also one major problem with the current autotimer.
5m for minutes does work, 5s for seconds does not. There it has to be 5 alone, without the s. Should also be fixes asap ;) thx

@thehik
Copy link

thehik commented Nov 29, 2016

I vote for converting the value to the item's type. In case that someone really needs an autotimer value of type string one could explicitly use quotes e.g.:
autotimer = 10 = "non-casted string"

However, there should be a unique solution for cycle, crontab and autotimer.

@msinn
Copy link
Member

msinn commented Dec 29, 2016

I am working on the 'casting' issue for autotimer and timer, because I am working on item.py anyhow. I am implementing the usage of items in the autotimer-attribute.

For the time beeing cycle (und crontab) stay as they are, because the handling for them is done in a complete other part of the source code.

@cstrassburg
Copy link
Member

Kannst du das bitte in einem separaten branch implementieren. Danke

@msinn msinn self-assigned this Dec 31, 2016
@msinn
Copy link
Member

msinn commented Jan 2, 2017

I have been able to fix the 'casting' issue for cycletoo.

@msinn
Copy link
Member

msinn commented Jan 21, 2017

Implemented for Version 1.3 and documented in wiki.

@msinn msinn closed this as completed Jan 21, 2017
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

6 participants