[WIP] New Vue Web UI #1933
[WIP] New Vue Web UI #1933
Conversation
|
I :) I'm learning web development for embedded applications and I'm testing preact and just curious about Vue. Can you explain in the PR message which will your strategy for build the project, also it is important to test the website, have different components and in the build create a single HTML with all styles and JS. How are you going to manage that? After some research using LitElement and preact, preact is way smaller, with the same application LitElement is 21Kb size and Preact 4Kb. I used Webpack to inline the css and JS code in a single HTML Thanks for contributing to that awesome website!! |
|
Build will be made by webpack so with a bit of configuration it will handle the minification of html/js/css and inlining them into one html which then will need to be gziped, it by default already packs all components/scripts into one app.js and css into one app.css so not much left really Never heard of preact but vue makes a lot of sense for single page UIs it's really easy to understand what's happening and having worked with react the component logic and writing is simpler (meaning more intuitive for the community to support and more lightweight as well) https://vuejs.org/v2/guide/comparison.html#Preact-and-Other-React-Like-Libraries 4kb is indeed a very small framework size, I wonder if they include all important features of react |
|
Yes, Vue was another alternative I studied but I've never used it before (I used react) and it was easy for me to create a small application to test preact, it includes the important features. Thank for your explanation 🎉 🎉 , I hope I can test Vue as well to see the comparison. |
Some previous attempts were here #970 and #1680 , but the way it was done is by updating PlatformIO SCons filelists. Recent platformio versions make it so that each environment has it's own scons file list, so if I were to add web files to the track list there it would always rebuild the UI when the firmware is built. It can also not do that and just call npm-build and hope it did something (but the question is now, how it would know about which modules it wants) Configuration should be easy enough to finally implement, but all the versions so far used to run gcc preprocessor and manually parse the result via regexp. I wonder if that could be avoided somehow and still show an expected build config. As you mentioned in the other issue, backend updates can go straight to the dev optionally / updating existing webui too. |
Refactoring ws api + Added repeater field to build templates
|
Little update: around 50% of the work is completed |
code/espurna/sensor.ino
Outdated
| sensor["humCorrection"] = _sensor_humidity_correction; | ||
| sensor["snsRead"] = _sensor_read_interval / 1000; | ||
| sensor["snsReport"] = _sensor_report_every; | ||
| sensor["snsSave"] = _sensor_save_every; |
There was a problem hiding this comment.
In this module, what are all this values and why are they not per magnitude or per sensor?
Lost me on this code
There was a problem hiding this comment.
Correction code makes more sense to have per sensor instead of globally, perhaps the assumption was that the device only uses a single sensor or sensors are all in the same place / have similar problems that require correction
Read / report values could go per sensor too, since those could have a different requirements. idk about saving, because the only use case right now is energy (which, btw, is saved per sensor as eneTotalN)
Units could be kept global? Is there a reason to have them specialized?
|
@mcspr Quite busy so don't have time to roll it up for beta testing yet There is a few settings I added on the interface that don't exist yet Under relays:
I think that's all I added, they are in the schema sent already but not used yet If you could implement those options on the .ino side that'd be amazing (on this PR or on dev up to you) I agree that the sensors could use a little refactoring but I am not too familiar with it, maybe first let's roll this out and then think on how to refactor the sensor to minimize feature change in this PR |
| schema.add("ip"); | ||
| schema.add("gw"); | ||
| schema.add("mask"); | ||
| schema.add("dns"); |
There was a problem hiding this comment.
Note that I was kind of doing the same thing, hardcoded 7 can also be (sizeof(keys)/sizeof(keys[0]))
I changed the behaviour of wifi settings, getSetting() will return hardcoded default. stored flag is to avoid removing hard-coded entry accidentally, since on ui reload it will appear again anyway
There was a problem hiding this comment.
I just didn't want to use copyFrom because it was removed from version 6 of the arduino Json lib (Using v5 but if ever we move away)
Also not sure but doesn't it double the ram usage for the array (define + clone, seems redundant)
Edit: Never mind, add also clones the value
I don't get the stored flag
Don't if (!getSetting({"ssid", index}, _wifiSSID(index)).length()) break; prevent it from ever being set to false anyway, by not sending wifis that are not set?
There was a problem hiding this comment.
See https://arduinojson.org/v6/doc/upgrade/#copyarray
When we have WIFI1_SSID=something, getsetting is never empty
There was a problem hiding this comment.
Oh I finally got it, so the WIFI can be hardcoded with defines and this flag tells us if it comes from the define or not
But can't they be overwritten? The defines acting just as a default value
There was a problem hiding this comment.
Yes, see code/espurna/wifi_config.h
They are hard-coded defaults, we can't erase them. It is still possible to "shadow" them, either by setting indexed setting to something else or setting "" empty value
There was a problem hiding this comment.
Okay so I got it all but then there might be an issue, if you overwrite the settings the delete button will still be disabled, but it shouldn't (it wouldn't technically delete it, but would reset it to default)
So maybe just changing the name of the button to 'Reset to default' instead of 'Delete' but the API wouldn't know the default value and would need to retrieve it again
Having a hardcoded flag sometimes complicates things more than it simpifies it :p
There was a problem hiding this comment.
Kind of like that. You can always hit reset-all button though, I guess there could be a specific 'reset wifi settings' one.
pre 1.14.2 versions read hardcoded settings on boot, overwriting runtime settings with those values, so it had the same problem only delayed until the next reset
There was a problem hiding this comment.
btw, not to speculate on code improvements regarding arduinojson:
#1957 (comment)
you can use it as a local library and build program on host. you can repeat the code from above for yourself, array size does not change when we copyfrom() or add individually, it only stores pointers since it is array of pointers.
There was a problem hiding this comment.
Yeah that's what I figured, so there is no difference between the two? It's a matter of personal preference then, taking into account that some schemas are variable with macros (defines), is it possible to create an array with macro conditional to copy from?
There was a problem hiding this comment.
True, effectively it would be the exact same code generated for v5:
https://github.com/bblanchon/ArduinoJson/blob/v5.13.4/src/ArduinoJson/JsonArray.hpp#L151
Slightly different for v6
https://github.com/bblanchon/ArduinoJson/blob/8f8c82d400a2e860e538eed8f92674330186f76f/src/ArduinoJson/Array/Utilities.hpp#L47
But [index] access is also available in v5 (not sure what empty values are though, nulls?)
|
idk if you are testing something, but what is up with newline removal of debug? why is it needed? |
|
Just experimenting |
*buttons? as: All of those are indexed, i.e. per button? |
code/espurna/relay.ino
Outdated
|
|
||
| schema.add("dblDl"); | ||
| schema.add("lngDl"); | ||
| schema.add("lngLngDl"); |
There was a problem hiding this comment.
re my previous comment, clicks & click delays are button settings. I'll add the specific part to the button code then
There was a problem hiding this comment.
Hmm didn't think about that since the relationship between buttons and relays is almost always 1/1 but there might be cases where buttons don't have a specific relay or vice versa..
In which case it might be a good idea to expose the settings on the button module and to do the relation later on
In the future, that could open the door for a device module where the buttons/leds/relays GPIO, manufacturer and device could be configured. That would allow for non hardcoded generic builds in pio where you could configure your buttons/relays/led on different GPIOs, edit the manufacturer and device freely where one build fits a lot of buttons/relays/leds only devices and you can just have a list/database of configurations online depending on the device (Eg: sonoff 1ch, 2ch, 3ch, touch, sv etc) to just import to the device
There was a problem hiding this comment.
I think I wanted to do buttons in dev branch first :>
Note on the b352284 - .ino should preferably be using constexpr funcs that return preprocessor vars, that way you already know the type.
edit: or like with delays - UL suffix will force the type to unsigned by default
But I agree on board ID thing, I don't think I want that after all. Same with migrate, only keeping that for now to reference keyvalues that were used before
There was a problem hiding this comment.
Yep I changed all that already :p
I'll be organizing some more stuff (a few settings to rename) and then start testing on a device to finish up the vue app,
For the board id it's not used anywhere and we already have the MANUFACTURER and DEVICE info, for the migration we can keep it (though it may take some build space)
I'll need to add a new config version and migrate some settings anyway so I'll use it
I'll add the buttons settings under the leds tabs and rename it Leds/Buttons
Avoid that clustered mess that is relay
There was a problem hiding this comment.
I tried some board stuff in the #1680, but was still different from the old v2 approach. Main idea was to support device 'sets' that consisted of key-value pairs and swap config based on that build-time ID. migrate.ino was also safe-guarded against overwriting, like it does right now.
Headers are finally fixed, so sourcing config/all.h with any external app is much more easier than it was in that PR.
My only concern is to not get too broad with modifications (while staying on the topic of Vue UI, see the title :). But, if that helps with an overall design of the UI, sure; better integrate the stuff now when it is less pain to change.
And as I was saying - I may break when adding the same thing to dev in the meantime, which will probably involve manual merging of stuff related to the configuration. Mostly how things are chained together and supporting indexed stuff at build time.
There was a problem hiding this comment.
Hmm for the migrate, does that mean it's not implemented yet? From what I see none of the settings set there are actually used yet..
(Eg relayGPIO, relayType, btnGPIO)
And it was actually an attempt to try what I suggested with the database of devices/settings except here it's hardcoded in the binary
With the pwa it will be possible to have a json file with the list of devices and config and have a modal on init to select the device and load the config at the same time you set the password, won't do it in this PR but to keep in mind
I'm trying to limit modification but I'm refactoring the whole websocket interface (eg: callbacks, schema etc), so I'd rather have some bases covered here than later and have all the breaking changes at once (for a v2 basically) so I'd prefer having all the settings at least added in the schema and add the functionality later
There was a problem hiding this comment.
Nah, forget about migrate, we already have device settings in the hardware.h + key association is implemented in each module separately instead of having it all in one place. It was a prototype, but there is no actual point of having it anymore.
The general idea from v2 & pr is to keep key-value pairs for each device, based on current MANUFACTURER_DEVICE flag (either all at once, or limited to user-specified set).
As this PR is still wip there is no point of really restricting yourself, but just keep in mind that I'd prefer to merge settings and API changes beforehand. Let's just see what works
code/espurna/button.ino
Outdated
| #if MQTT_SUPPORT | ||
|
|
||
| void buttonMQTT(unsigned char id, uint8_t event) { | ||
| char payload[4] = {0}; | ||
| itoa(event, payload, 10); | ||
| mqttSend(MQTT_TOPIC_BUTTON, id, payload, false, false); // 1st bool = force, 2nd = retain | ||
| mqttSend(MQTT_TOPIC_BUTTON, id, payload, false, getSetting({"btnMqttRetain", id})); // 1st bool = force, 2nd = retain |
There was a problem hiding this comment.
Note that 2nd argument's type is always mapped to the return type, getSetting<...>() syntax can be avoided.
(preferably PREPROCESS_FLAGS are adjusted to be the correct type directly instead of casting)
getSetting(key) always returns String obj as-is from the underlying storage, since we don't know the type beforehand
There was a problem hiding this comment.
So getSetting({"btnMqttRetain", id}, false) is preferred over getSetting<bool>({"btnMqttRetain", id}) ?
There was a problem hiding this comment.
Yes, since there is no getSetting<bool>(key)
Which could be implemented in theory, but instead of explicit false then you'd need to remember what exactly is the default of the type that you want.
# Conflicts: # code/espurna/board.h # code/espurna/board.ino # code/espurna/button.h # code/espurna/button.ino # code/espurna/button_config.h # code/espurna/config/dependencies.h # code/espurna/config/general.h # code/espurna/homeassistant.ino # code/espurna/led.ino # code/espurna/migrate.ino # code/espurna/relay.ino # code/espurna/sensor.ino # code/espurna/ws.ino # code/platformio.ini
| @@ -214,25 +214,17 @@ void _debugSendInternal(const char * message, bool add_timestamp) { | |||
|
|
|||
| #if DEBUG_WEB_SUPPORT | |||
|
|
|||
| void _debugWebSocketOnAction(uint32_t client_id, const char * action, JsonObject& data) { | |||
|
While you are looking at HA module, check out dev...mcspr:ha/ext-structs Idk if this is still relevant, but the idea is to put string-string container on top and generate either yaml of json from that. c++ is weird though, since we probably need std::unordered_map for hashmap instead of std::map, b/c it uses rbtree + binary search comparisons (and idk how well that works huh) |
This pull request is a work in progress and will in term create a new user interface using Vue replacing the old jquery lib and templates logic and dividing the size of the html build by 2
I am estimating a 150Kb image non gziped, 40Kb gziped, will depend a lot on build flags for integrations, I guess we'll see (In comparison the current image is 213Kb, 58Kb gziped)
Components:
Organization
Javascript
Css / styles
Build process
HTML
General
Core
Tests
PWA