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

Allow webbutton up to 32 #19580

Merged
merged 2 commits into from
Sep 29, 2023
Merged

Conversation

barbudor
Copy link
Contributor

@barbudor barbudor commented Sep 23, 2023

Description:

Related issue (if applicable): fixes #19324

My proposition to support WebButton up to 32
No data stored in Settings => must use autoexec.bat or on system#init rule to set on each reboot
Usage:

  • adds 176 B of flash
  • adds 64 B of RAM

(Note: I have tested an alternative with +8 B of RAM instead of +64 but +224 B of flash, I believe it's better to optimize flash usage)

There's no #define. Not easy to add due to MAX_BUTTON_TEXT being defined as a const in tasmota.h (not overridable)

image

Checklist:

  • The pull request is done against the latest development branch
  • Only relevant files were touched
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • The code change is tested and works with Tasmota core ESP8266 V.2.7.4.9
  • The code change is tested and works with Tasmota core ESP32 V.2.0.13
  • I accept the CLA.

NOTE: The code change must pass CI tests. Your PR cannot be merged unless tests pass

@arendst
Copy link
Owner

arendst commented Sep 23, 2023

Thx for the idea. Give me time to evaluate.

@arendst arendst self-assigned this Sep 28, 2023
@arendst
Copy link
Owner

arendst commented Sep 28, 2023

Ingenious!! You extend the webbuttons using a command like WebButton17 OMG in autoexec.bat which stores the value in RAM.

Let me think how to extend this functionality "easily" for other user requested functionality.

@barbudor
Copy link
Contributor Author

Hi @arendst ,
Except for core features, I'm much in favor of putting things that are not often used elsewhere than Settings.
More drastically, settings for a feature that is not used (or even not in the build) should not take up room.

You may have suggested something based on JSON but that means adding JSON parsing to every driver and we would likely loose in code-bytes what we would have saved in settings-bytes.
So why not using something that is already there : the command parser and autoexec.bat
Generalizing a small file system, even on 1MB build could be very helpfull with that
And using 1 single file (autoexec.bat) vs 1 file per driver, is using more efficiently the flash estate.

One point is that in that case, settings changed in the console or through MQTT are not automatically saved (vs automatic save of Settings). This can be seen as a disadvantage as user need to remind to update his autoexec.bat, but that could also be an advantage when tinkering as a simple restart reverts to the last-known-good state.

@arendst arendst merged commit 335e18e into arendst:development Sep 29, 2023
63 checks passed
dilyanpalauzov pushed a commit to dilyanpalauzov/Tasmota that referenced this pull request Oct 1, 2023
* Allow webbutton up to 32

* use named const
@barbudor barbudor deleted the webbutton_17-32 branch November 29, 2023 21:15
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

Successfully merging this pull request may close these issues.

2 participants