search

letscontrolit / ESPEasy

Easy MultiSensor device based on ESP8266/ESP32
http://www.espeasy.com
Other
3.25k stars 2.2k forks source link

[Question] Duplicate Task Names #1993

Closed clumsy-stefan closed 1 year ago

clumsy-stefan commented 5 years ago

More a question than an issue actually...

I know that since some time there is a check for duplicate Task names (which should be avoided, I know), however I use it to have all system information in one "virtual" device at the end.

When using duplicate names it shows an error/warnign (that's ok), but now it seems that when adding or changing tasks with a duplicate name it just clears the value name of that task and does not save the complete task.

Not sure if that's intended like this, but at least the behaviour has changed.

can anyone enlighten me?

TD-er commented 5 years ago

There is something a bit strange in the code, regarding the value names of a task. Not sure if you refer to the value names, or the task names?

If the first value name is empty (first byte is 0), then all value names will be considered not set. If it is not set, it will load the default names. So if the rest is not set, and you fill in the first one, it may trigger undefined behavior. Also the other way around, if you empty the first value name, I don't know what will happen to the rest.

clumsy-stefan commented 5 years ago

I really mean task names!

I have 4 tasks named 'sysinfo' with each one value (rssi, load, men,,uptime)

Grovkillen commented 5 years ago

Regarding that particular plugin "Generic - System info" we should consider adding the four most used values in the plugin. So an option would then be "default" = RSSI, LOAD, MEM, UPTIME and "single" = you pick what value to use.

Grovkillen commented 5 years ago

Maybe single should be the default for backwards compatibility. And the other one is called "Load values" or something?

Grovkillen commented 5 years ago

I have 4 tasks named 'sysinfo' with each one value (rssi, load, men,,uptime)

I really think this is an normal use case which we should take care of.

TD-er commented 5 years ago

You may want to have a look at the Eastron plugin I changed last week. (in Testing) That plugin can give 10 different values, but I made it selectable what value should be used at what position.

For Domoticz, sending several values which are not really related can be an issue. So maybe we should add an option to add an IDX to each output value and send separate messages to Domoticz when these differ per output value. This would solve a lot of other issues with plugins an Domoticz I guess.

N.B. these separated IDX values should be only for a number of plugins which collect parameter which should be split to be handled in Domoticz. (e.g. SDS011)

clumsy-stefan commented 5 years ago

As @Grovkillen did, I'd support the idea of having one task with for selectable values!

I don't use Domoticz, so I can't comment on that. I use it with the fhem controller.

Still I think it should not clear value names when using duplicate task names! I'd consider this a big...

TD-er commented 5 years ago

Still I think it should not clear value names when using duplicate task names! I'd consider this a big...

I agree on that :)

clumsy-stefan commented 5 years ago

Still I think it should not clear value names when using duplicate task names! I'd consider this a big...

a bug actually, but not really big (I hate autocorrection)

I agree on that :)

great ;)

clumsy-stefan commented 5 years ago

I know it's not a real solution, but for me currently a workaround. changing line 501 & 507 in misc.ino to // return err; and err += LoadTaskSettings(taskIndex); respectively makes it possible again to at least save the settings without clearing the value name if the taskname is duplicate!

Of course I'd appreciate a final solution on that ;)

clumsy-stefan commented 5 years ago

@uzi18 patch #2010 solves my issue here partially. if above change (allowing duplicate task names but warn it) could also be included this issue could be closed from my point of view.

I can make a pull request of it, if needed..

TD-er commented 4 years ago

The bug/issue is actually about the rules engine not being able to find a matching rule when a value is updated from a 2nd plugin with the same name.

Events with an update of variable bar2 will not be processed in the rules.

clumsy-stefan commented 4 years ago

Thanks for checking. Bug or feature(-request) in your opinion?

TD-er commented 4 years ago

Thanks for checking. Bug or feature(-request) in your opinion?

Both. Strictly speaking we have a requirement for unique task names, but since we don't enforce it, we must make sure it can be used like this in rules.

clumsy-stefan commented 4 years ago

Why is this requirement or where does it come from? I think this is probably an MQTT requirement, but offline nodes or nodes with other controllers are absolutely happy with duplicate task names..

but I agree, as it's allowed it should work correctly ;)

TD-er commented 4 years ago

ESPEasy core has already been made a lot more flexible, but in the (recent) past it was a true design with only 1-to-1 relation with logic for it spread all over the code. So now it is a bit less difficult to change, but still will involve quite some code changes to make it work.

clumsy-stefan commented 4 years ago

relying on uniqueness in names (especially when you have indexes/numbers) is generally not the best idea in programming, or I'm I wrong here?

In ESPEasy uniqueness could be done through a tasknr-variablenr combination, these are always unique...

TD-er commented 4 years ago

In ESPEasy uniqueness could be done through a tasknr-variablenr combination, these are always unique...

That would only work if the task numbers had to be used in the rules to address a variable. But we work on the names, not indices.

tonhuisman commented 1 year ago

This issue is no longer relevant, and can be closed.