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

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

Open
brunetton opened this issue Nov 7, 2024 · 3 comments
Labels

Comments

@brunetton
Copy link

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
Copy link
Collaborator

mcspr commented Nov 7, 2024

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

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

// 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

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

@brunetton
Copy link
Author

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
Copy link
Collaborator

mcspr commented Nov 8, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants