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

Alternative to current Web UI #27

Closed
TerryE opened this issue Apr 3, 2018 · 8 comments
Closed

Alternative to current Web UI #27

TerryE opened this issue Apr 3, 2018 · 8 comments

Comments

@TerryE
Copy link

TerryE commented Apr 3, 2018

Marcel,

Things are getting very complicated on the cloud builder UI front. As well as the obvious modules selection options, we now have a host of user_config.h options that can be tweeked. Instead of trying to provide a web UI to do that various check boxes and number options, why not just have an options to allow users up upload their own user_config.h?

@marcelstoer
Copy link
Owner

Things are getting very complicated on the cloud builder UI front.

What exactly do you mean with "getting very complicated"? Is there anything that will fundamentally change soon that I missed? AFAICT both user_config.h and the options GUI in the cloud builder remained largely unchanged in the past 2.5 years.

why not just have an options to allow users up upload their own user_config.h?

The security implications could be dealt with, they're not a real issue.

To me it's all about target audience. I built the service exactly for people who don't want to learn about the host of options - most of which we didn't document anyway. The few options the web GUI actually offers are what I was asked to provide by the users. I'd be more than happy to extend it with anything there is real demand for. My thinking has so far been that anyone who needs to dig into user_config.h is probably willing to go the extra mile (actually it's rather a yard) and use the Docker image.

Things will get a bit more complicated once the options for the different branches diverge. Implementing support for ESP32, which I think is getting absolutely essential, needs to solve that.

@TerryE
Copy link
Author

TerryE commented Apr 3, 2018

Marcel, We keep adding extra features that chip away at RAM availability -- even if the app doesn't use the feature. This is worse when ICACHE_RAM_ATTR is involved since there is precious little of this available on builds. I feel that our default user builds should be lean and clean, but whatever, with LFS etc, most users might want to tweak these.

Variable Discussion
DEVELOPMENT_TOOLS
DEVELOPMENT_USE_GDB
DEVELOPMENT_BREAK_
ON_STARTUP_PIN
Needed for development, but can be ignored for cloud builds.
FLASH_*
BIT_RATE_*
Similarly probably only of interest to developers.
LUA_FLASH_STORE n Must be set to use LFS. This allows you say a 4-10× increase in size of Lua applciation that you can run, and also allows you to code in a simpler more transparent style because code resources aren't taking up RAM anyway.
LUA_PACK_TVALUES_ENABLE
LUA_NUMBER_INTEGRAL
These together dictate whether the firmware is using 8, 12 or 16 byte TValues, which has a major inpact on RAM footprint.
BUILD_SPIFFS
BUILD_FATFS
FS_OBJ_NAME_LEN n
Most developers don't touch these.
GPIO_INTERRUPT_ENABLE
GPIO_INTERRUPT_HOOK_ENABLE
Required for applications which use GPIO triggers. A RAM and runtime over head for the majority of applications that don't, so need commenting out.
PMSLEEP_ENABLE
TIMER_SUSPEND_ENABLE
Very useful for some applications which use node.sleep(). A RAM and runtime over head for the majority of applications that don't, so need commenting out.
SPIFFS_MAX_FILESYSTEM_SIZE n
SPIFFS_FIXED_LOCATION n
SPIFFS_MAX_OPEN_FILES n
SPIFFS_CACHE b
Using a maximum SPIFFS on a 4Mbyte module is just a killer. My strong advice is that developers size their images to a sensible size matched to their application and use spiffsimg and esptool to reprovision the FS as needed. Reducing the 3rd option saves RAM if, say, your app only opens one file once. Disabling the cache saves RAM, but is only something that I'd suggest if the SPIFFS is only used read-only.
SSL_BUFFER_SIZE n
CLIENT_SSL_ENABLE
MD2_ENABLE
SHA2_ENABLE
SSL / security related options. Disabling / tuning these saves both RAM and Flash. I nearly always do this.
WIFI_SMART_ENABLE
WIFI_SDK_EVENT_MONITOR_ENABLE
WIFI_EVENT_MONITOR_DISCONNECT_
REASON_LIST_ENABLE
Nice additional functionality if you need it, but most users don't (or in my case use native Lua equivalents). Disabling these saves RAM and flash.
WIFI_STA_HOSTNAME "name"
WIFI_STA_HOSTNAME_APPEND_MAC
You either need this or you don't.
STRBUF_DEFAULT_INCREMENT n 32 is too small IMO, so I always up this to 64.
ENDUSER_SETUP_AP_SSID "SSID Name" Why this is a user_config option escapes me, but if you use ENDUSER_SETUP, then you will probably want tot change this.

@marcelstoer
Copy link
Owner

Thanks for the great listing Terry!

For me the options fall into one of these categories:

  1. brand new, not even in firmware dev yet (LFS, TValue)
  2. ignore for cloud builder
  3. commented out anyway (PMSLEEP_ENABLE, TIMER_SUSPEND_ENABLE, MD2_ENABLE, WIFI_SMART_ENABLE)
  4. I disagree with you that most developers need to change them (WIFI_STA_HOSTNAME, ENDUSER_SETUP_AP_SSID)

What remains is that the cloud build should maybe offer to disable WIFI_SDK_EVENT_MONITOR_ENABLE and GPIO_INTERRUPT_ENABLE even though I would leave both enabled by default.

Of course, LFS will have a major impact and the cloud builder needs to support it.

Just wish my day had 48h.

@cwrseck
Copy link

cwrseck commented Apr 15, 2018

Initially I used the cloud builder for NodeMCU, and moved to local builds only to fix a bug in ow.c. I think it gives quite enough flexibility for the first-time user, though LFS will certainly require an update to the interface. And building from scratch, anyway on Linux, is very straightforward - just git fetch and make.

Adding the table above to the FAQ would be very helpful; I hadn't understood, or heard of, most of those options.

Will

@TerryE
Copy link
Author

TerryE commented Apr 15, 2018

@cwrseck Will, have you seen my reference to this version of the user_config.h which adds decent self documentation to help developers such as yourself.

@cwrseck
Copy link

cwrseck commented Apr 18, 2018

No, I'm using the 2.1 and 2.2 releases. The new user_config.h looks to b a considerable improvement from where I'm standing; really useful information.

Thanks - Will

@TerryE
Copy link
Author

TerryE commented Apr 30, 2018

Marcel, I would like the CBS to support LFS builds at the same time that we pull this into dev, which should be in the next week or so. Given this can I again request that we have a menu option to support cloud builds with the following parameters:

Variable Option
LUA_FLASH_STORE n This must be set to use LFS. Even a selection (disabled, 32Kb, 64Kb, 94Kb) would be a good start.
LUA_PACK_TVALUES This option dictates whether floating point builds use 12 or 16 byte TValues, and this in effect pretty much increases RAM availability by ~25% without any performance hit. This should be a check box or turned on by default for floating point dev builds.
SPIFFS_MAX_FILESYSTEM_SIZE n
SPIFFS_FIXED_LOCATION n
Fixed location SPIFFS images are pretty much essential for rapid spiffsimg generation. I suggest that you allow users to optionally set the max filesystem size in multiples of 64Kb and set the fixed location to 0x100000 if the size is set.

Lastly as well as the firmware bin files, you should also document how to download esptool.py, luac.cross and spiffsimg files.

Thanks Terry

@marcelstoer
Copy link
Owner

Terry, I understand your motivation behind this request and I agree the cloud builder should support LFS. However, I've got currently so little extra time that I can't do it all alone. Let me make a proposal.

  • This issue originally requested "Alternative to current Web UI". This I won't implement. Can you close it and start a new one with just your last comment?
  • Have you added your improved user_config.h from the Gist to your LFS branch so it'll land on dev as well? If not, could you please do that?
  • As for LUA_PACK_TVALUES doesn't your "..turned on by default.." indicate it should actually be turned on by default in user_config.h? I don't think the cloud builder should offer this option. If 98% of the users won't need to tinker with it then I'd rather not bother them.
  • If you or someone else provides a PR for the build scripts I can promise to adjust the UI accordingly. The process is actually quite simply (I'm sure analyzed it already 😉 ):
    • All options are collected into build.config. That's what the UI pushes to the repo for every build.
    • They're all made available as X_ env variables.
    • Then a dedicated Bash script is triggered to process each one thereby modifying the various .h files. Warning: my Bash skills are not the best, and they were much worse 2y ago. Furthermore, some commands are more complicated than necessary because they need to work both on Linux for Travis and on Mac for my local testbed.
  • I don't understand what you mean by "you should also document how to download esptool.py, luac.cross and spiffsimg files." sorry.

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

3 participants