Schedules - UI should warn when one or more schedules have incorrect syntaxes

[brunetton]

Device

itead-sonoff-dual-r2

Version

1.18.0-gita518080a+github240830

Bug description

I was very happy to see that #2417 have been implemented 🥳

Today it's the day when I have to review my automatic door schedules. So I read the doc and tried to use combinations of months and SUNSET and SUNRISE keywords.

But I've absolutely no idea if my schedules strings are correct or not. The only way to have an answer is to wait for ... months :s

I presume that the string is parsed internally and if there are errors the information is available somewhere ?

It would be really useful to let user be aware of errors on save. At least instead of the "Changes saved" alert, it would be great to have "At least one schedule have an error". Then user can search and find the error.

Thanks !

Steps to reproduce

No response

Build tools used

No response

Any relevant log output (when available)

No response

Decoded stack trace (when available)

No response

[mcspr]

I presume that the string is parsed internally and if there are errors the information is available somewhere ?

Internally, yes, but only as a yes / no answer. Can point to the specific space-separated chunk, but would have to update the returned struct in that case to replace bool ok with e.g. pointer to the last parsed chunk and some kind of friendly error string

espurna/code/espurna/scheduler_time.re

Lines 973 to 977 in 79fa004

	// DATE " " WDS " " TIME " " KW
	const auto spaces = std::count(view.begin(), view.end(), ' ');
	if (spaces > 3) {
	return out;
	}

espurna/code/espurna/scheduler_time.re

Lines 1016 to 1036 in 79fa004

	// expect one of each element, plus optional keyword
	if (parsed != (1 + spaces)) {
	return out;
	}
	// do not want both time and sun{rise,set}
	if (want_sunrise_sunset(out.time) && parsed_time) {
	return out;
	}
	out.ok = parsed_date
	|| parsed_weekdays
	|| parsed_time
	|| parsed_keyword;
	if (out.ok && !parsed_time && !want_sunrise_sunset(out.time)) {
	out.time.hour = 0b1;
	out.time.minute = 0b1;
	}
	return out;

Otherwise, as a temporary workaround, systemd-analyze calendar ... would do the trick. Obviously, excluding the extensions added here
https://www.freedesktop.org/software/systemd/man/latest/systemd-analyze.html

1.18.0-gita518080a+github240830

As mentioned in the previous issue - you'd have to build it locally first with this option enabled

espurna/code/espurna/config/general.h

Line 1419 in 79fa004

#define SCHEDULER_SUN_SUPPORT 0 // Support 'sunrise' and 'sunset' time keywords

[brunetton]

I presume that the string is parsed internally and if there are errors the information is available somewhere ?

Internally, yes, but only as a yes / no answer.

This would be perfect ! I could have noticed by myself that SUNRISE is not valid with the default build, without having to wait until this morning to find that the shutter hadn't opened :s

As mentioned in the previous issue - you'd have to build it locally first with this option enabled

I guessed that this morning when I seen that the scheduler wasn't working. But I thought the source of truth was in the doc: https://github.com/xoseperez/espurna/wiki/Scheduler "Build scheduler module, 1 by default (0 to disable)"

Usually (I mean in all firmwares and softwares I've seen) the assets are built with the default provided configuration. And users who wants to activate/deactivate some functionalities can do it in their own copies of the configuration.

[mcspr]

I guessed that this morning when I seen that the scheduler wasn't working. But I thought the source of truth was in the doc: https://github.com/xoseperez/espurna/wiki/Scheduler "Build scheduler module, 1 by default (0 to disable)"

Looks like the same issue as NTP one, wrong column used for runtime and build settings? Adjusted rn

Usually (I mean in all firmwares and softwares I've seen) the assets are built with the default provided configuration. And users who wants to activate/deactivate some functionalities can do it in their own copies of the configuration.

True, but .bin are provided as a convenience for existing devices with sort-of default fits-all config. I hope it is understandable that any code added is not free, and takes more and more space, the more things are added.

I would (very) strongly suggest to take a look at PIO (CLI or through VSCode extension) and just build locally. This also simplifies configuration on per-site basis by a lot

[mcspr]

^ chain of commits above, plus c8ebbbd to cancel validity

Experimenting with possible checks & notifications.

• schedule check ... terminal call for manual checks
• debug log when settings reload (terminal reload, webui 'save' button)
• webui notification pointing at the input elem

But, unless I am missing some obvious web api call, js cannot display any pre-localized string from the built-in set of translations. e.g. I cannot tell it to say 'value is invalid' in UI language. Which is pretty annoying, considering every other validity message is translated

[brunetton]

Sound great :) Thank you, I'll play with it when I've time !