Call commands from the Web UI about ems-esp32

[MichaelDvP]

With the additions of emsesp/EMS-ESP#506 heap is ~2k lower, not enough to reload web-page if telnet is open on esp8266. I think you can cross out the second option (telnet on web).

[proddy]

I was thinking when you select a device in the EMS Devices web page, it shows the value below and below that show a new form where you can choose a command from a drop-down, a value, and an id. It's a luxury feature and not high-priority but it does kind of round of the Web UI as being a fully functional dashboard & command center.

[proddy]

@MichaelDvP do you still think its a good idea?

[MichaelDvP]

With the API it's possible to do a command from web, but not very comfortable in the browser-url.
A input-line in web for a call command would be fine, or like scan-devices a button for a dialogbox to enter a command.

[proddy]

What I dislike about the current API is that it's insecure (if you enable it in the Settings) so anyone can send write commands to the EMS-ESP. Having these calls from the Web UI offers some protection and only accessible via a user with the Administrator role. I like the idea of a button which pops up a dialog window where you select the target (system, boiler, thermostat etc), command (free text) and data and id (both optional). The only worry I have is that we're close to 92% progmem flash so not much space left for new web code.

The Web API really needs to use the JWT token for added security. Something to add onto the list for later.

[proddy]

The better option I'm thinking is

• remove the Device ID, Version and Product ID from the 'Devices' web page as this is too techie and can be hidden in a mouseover tooltip
• Have an editable table so keynames that have an associated command (like flowtemp) would be highlighted, the user can click and type in a new value and this will be sent as a command to EMS-ESP
• at the same time have a real RESTful API like ems-esp/api/// instead of posting variables with & and ?. I'm using the GitHub API as a reference. The payload will always be JSON.

[proddy]

I did the first implementation of this. Each device value can have an associated command (now I see why we should align the mqtt names with the commands!), and this is shown with a little pencil. Clicking it opens a dialog where a new value can be set and the command sent to EMS-ESP.

I also cleaned up the page a little. The version, productID, deviceID are hidden in the ToolTip hover. And the device values table is denser. @MichaelDvP @bbqkees - let me know what you think, plus names and other UI things that look weird to you.

It's all in the branch called ft_webcallcmd. You can run it locally on a PC by going to the interface folder and typing npm run dev.

[MichaelDvP]

Very nice, commands working great, but they are accessible and working also from guest account. Was this intended?

The device names are now looking a bit unaligned to the sensors and values. Can you also move the sensor id to a tooltip? And device-names better right aligned.

There are some commands with no published value, like thermostat party, pause, holiday, switchtime. The settings are very rarely used. The first three could be added to publish, but switchtimes are a set of 42 values building a program, the command sets a single switchtime, i think there is no way to implement this to web.
One (maybe a few) other command(s) needs a bit tuning (like maintenance / maintenancetype)

[MichaelDvP]

Using register _cmd and register_device_value and ensure that both have identical texts and set the has_cmd is a bit error-prone.

What do you think about combining the commands in emsdevice.cpp and use only a single call for value and command?

void EMSdevice::register_device_value(uint8_t tag, void * value_p, uint8_t type, const __FlashStringHelper * const * options, const __FlashStringHelper * const * name, uint8_t uom, cmdfunction_p f) {
    register_device_value(tag, value_p, type, options, name[0], name[1], uom, (f != nullptr));
    if (f != nullptr) {
        if (tag >= TAG_HC1 && tag <= TAG_HC4) {
            Command::add(device_type_, name[0], f, FLAG_HC);
        } else {
            Command::add(device_type_, name[0], f, 0);
        }
    }
}

and register the values without command as

register_device_value(TAG_THERMOSTAT_DATA, &errorCode_, DeviceValueType::TEXT, nullptr, FL_(errorCode), DeviceValueUOM::NONE);

and with command and HC-flag as

register_device_value(tag, &hc->mode, DeviceValueType::ENUM, FL_(enum_mode3), FL_(mode), DeviceValueUOM::NONE, MAKE_CF_CB(set_mode));

Then there are only a few register_cmd left with no published value, but the device values/commands are all in sync.

[proddy]

Yes! I was going to do that next. There only needs to be a single function. This will ensure there is no mis-match between the command names and the MQTT key names.

[MichaelDvP]

Should i make a PR to the command branch for this suggestion (i'm testing it actually) or do you have a better idea?

[proddy]

yes please push the change straight into the ft_webcallcmd branch (or do a PR). I've been using this branch for other minor changes too.

[proddy]

@MichaelDvP ok to merge this back into dev? Any feedback on the UI changes and layout?

[MichaelDvP]

Yes, it works great.
As metioned in layout i would prefer the device brand/name to be alligned right, but not important.

[proddy]

right aligning like this?

that would look funny on large screens no?

[MichaelDvP]

The values for parameters and sensors are also right aligned, For me it looks better, but i think there are different opinions.

[MichaelDvP]

BTW: with the extra commands the json gets to small, my boiler cuts after disinfecting. For systems with 4 hc in thermostat we should enlarge to EMSESP_JSON_SIZE_XXLARGE_DYN 8192 (6k is ok for my boiler)

[proddy]

I made the changes