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

MAX_RULE_VARS and (maybe) MAX_RULE_MEMS limits. #4933

Closed
benzino77 opened this issue Jan 15, 2019 · 9 comments
Closed

MAX_RULE_VARS and (maybe) MAX_RULE_MEMS limits. #4933

benzino77 opened this issue Jan 15, 2019 · 9 comments
Labels
enhancement Type - Enhancement that will be worked on fixed Result - The work on the issue has ended

Comments

@benzino77
Copy link

Have you look for this feature in other issues and in the wiki?
yes.
Is your feature request related to a problem? Please describe.
Starting from version 6.1.1.9 (this commit) there was a possibility to define own MAX_RULE_VARS and MAX_RULE_TIMERS in user_config_override.h. So one could (reasonably, having in mind that it consumes memory) define his own values for those parameters. Starting from one of the latest dev release (this commit) there is new rules trigger introduced and a limit to have maximum 8 MEMS or VARS.

Describe the solution you'd like

I would like to have the ability to define 16 VARS without compiler errors.
I had a conversation with Adrian on Discord, and he explained me that changing the limit for VARS could be rather simple, and it will consume another byte of memory (uint16_t instead of uint8_t for vars_event). He also, very clearly, explained me that changing MAX_RULE_MEMS is not so simple (because there have to be reserved another byte in EEPROM, which can leads to compatibility problems during update) and only @arendst can make that decision and decide how to do it - so I'm asking Theo to consider that also.

Describe alternatives you've considered
I don't have one.

Additional context
Significant majority of Tasmota users, just using precompiled bins. Those who compile the binaries on their own, realize that some changes can lead to larger memory consumption (changing number of vars, rule timers, etc.)... at least I want to believe in that ;)

(Please, remember to close the issue when the problem has been addressed)
I will.

@arendst
Copy link
Owner

arendst commented Jan 15, 2019

Ok got it. It was set to 8 for both Vars and Mems as Adrian said because of the uint8_t. Mems will stay at 5 as they are stored in flash using 10 bytes each as a number is stored as text to ease string compares.

I'll change Vars to max 16 using uint16_t.

arendst added a commit that referenced this issue Jan 15, 2019
Fix allowable MAX_RULE_VARS to 16 (#4933)
@arendst arendst added the fixed Result - The work on the issue has ended label Jan 15, 2019
@benzino77
Copy link
Author

Great! Thanks Theo!
Question: Why have you changed MAX_RULE_MEMS to 5 from 8? Was it a mistake and it should be 5 from the beginning, because there are 50bytes reserved in flash for that or there is other reason?

@arendst
Copy link
Owner

arendst commented Jan 15, 2019

MAX_RULE_MEMS has always been 5 and that's what has been reserved in flash. It was never supposed to change.

@ascillato2 ascillato2 added the enhancement Type - Enhancement that will be worked on label Jan 15, 2019
@benzino77
Copy link
Author

Thanks again for quick answer!
One more question: what will happen if I change the MAX_RULE_MEMS to lets say 8 or 16 (changing mems_event to uint16_t in last case)? The device which will be flashed with new firmware will lost the previous configuration or something worse will happen?

@ascillato
Copy link
Contributor

ascillato commented Jan 15, 2019

You need to move the rest of the config in settings.h to modify the memory map. MEMs is not just a uint16_t, it is also every MEM. 16 MEMs is A LOT of memory on EEPROM. So, if you do that all your config will be scrambled. Not backwards compatible.

@benzino77
Copy link
Author

benzino77 commented Jan 16, 2019

Forgetting about backward compatibility will it work or there is no chance for that? I'm asking for a case when I have a "green field" - new device (or erased with esptool via serial connection), flashed with Tasmota with those "strange" settings (MAX_RULE_MEMS >5) will it work or the flash will be scrambled and there is no chance for it to work? It is just "academical" question - I'm just curious.

Thanks again for answering my not related questions (16 VARS is good enough for me)! My feature/change request was fulfilled in couple of hours - this is amazing!

PS.
@andrethomas is not a part of this discussion, but his MCP230xx driver pushed Tasmota (using amazing, constantly developed Rules) to completely new level. If he implement his idea regarding using Arduino, as "generic" I2C slave for Tasmota (I know it is just an idea for now) Elon Musk will have to send another Tesla in space, this time without David Bowie but with Tasmota on board ;)

@arendst
Copy link
Owner

arendst commented Jan 16, 2019

Making the requested changes will enlarge the settings struc by 110 bytes and move the rules area down approaching the undocumented eeprom save area but still within the available flash page.

So it could work.

@benzino77
Copy link
Author

Thanks again Theo!
I used to say in such situations : "Niech twoja fura będzie zawsze zatankowana do pełna, a komórka ma pełen zasięg LTE" which can be translated to: "May your car be always full fuel, and the phone has full LTE coverage" :)

@andrethomas
Copy link
Contributor

@benzino77

If he implement his idea regarding using Arduino, as "generic" I2C slave for Tasmota

Will get there eventually - just not a priority at the moment... also first week back at office so things are going to be hectic for the next few weeks... to make matters worse work is also about dealing with PR's and PoC's so I also need to find time in between as to differentiate between work and hobby :)

gemu2015 pushed a commit to gemu2015/Sonoff-Tasmota that referenced this issue Jan 27, 2019
Fix allowable MAX_RULE_VARS to 16 (arendst#4933)
arendst added a commit that referenced this issue Dec 22, 2019
Change max number of rule ``Mem``s from 5 to 16 (#4933)
arendst added a commit that referenced this issue Dec 22, 2019
Change max number of rule ``Var``s from 5 to 16 (#4933)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Type - Enhancement that will be worked on fixed Result - The work on the issue has ended
Projects
None yet
Development

No branches or pull requests

5 participants