Improve config validation

[ROBOT0-VT]

Provided finer validation for configuration options

Reworked the run.sh script to use bashio to pull configs, allowing for the new validation changes

Added user-friendly config strings in translations/en.json

Minor general cleanup

[ROBOT0-VT]

WARNING: The conversion of integer options to boolean ones is a breaking change, requiring existing user configurations to be changed/saved with the new toggles before use.

I have not included validation code in run.sh to account for this.

[rrooggiieerr]

The Hass.io Access Point add-on still uses an old style add-on configuration format. The current format should be in YAML instead of JSON. Within the YAML you can also define validation rules. Maybe it's a good idea to migrate to the new format instead.

https://developers.home-assistant.io/docs/add-ons

[ROBOT0-VT]

Happy to do so, just didn't want to add too much clutter to the commits/PR. It's a simple one-to-one conversion, it's the same validation rules for both formats.

It'll be exactly the same structure and data, just in yaml format instead of json

[ROBOT0-VT]

Actually since all valid JSON is valid YAML, I could just rename it to config.yaml and call it a day 😂

[rrooggiieerr]

@ROBOT0-VT first of all, I never took the effort to convert this project to the latest YAML format, so thumbs up for that. Having said that, valid JSON is not valid YAML. Having a look at your proposed config.yaml I think you already figured that out.

I'm happy to give your modifications a try

[ROBOT0-VT]

Alrighty, latest branch frickery has fixed issues with the YAML conversion, go ahead and test if you'd like.

BTW, you can just slap plain JSON into a YAML file and it will interpret it correctly, so long as you put the document separator (---) at the top. I'm not so lazy as to do that and call it a day, but it's absolutely something you can do!

[ROBOT0-VT]

Alrighty, I pushed a change to allow older configs, turning this PR into a non-breaking change. No need to check the version, we can just convert old integer configs to boolean values (which I've done).

I've also changed the version to 0.5.0, as I'd consider such a large refactor a minor change, rather than a patch

[rrooggiieerr]

@ROBOT0-VT, I just installed your branch of the add-on, have to do a bit more testing, but one thing I'm noticing is that the numeric debug option can be any value including negative, but looking at the run.sh only the values 0 and 1 are used. I'm not sure if you should make them boolean or keep them numeric. If you keep them numeric other log levels could be introduced later. Anyways, the input validation should be adjusted for the current values.

Also the client DNS address can be anything, while only valid IP addresses should be allowed

Password length is allowed to be less than 8 while 8 is the minimum length for a WPA2 password

[ROBOT0-VT]

I'm not touching debug logging in this PR, that's a thread that needs to be pulled in its own time. AFAIR, 0 means debug logging disabled, 1 means debug logging enabled, and 2 means debug logging plus debug mode for hostapd.

Ah, I was filtering DNS server for domain names, not IPs 🤦 Not realising that you need DNS before you can get a domain name...

Unfortunately I can't control validation for passwords from the UI, as HASSOS doesn't accommodate that. I can add a fatal error in the code to handle it though.

[rrooggiieerr]

You could use a regex to validate it's length, something like match(^.{8,}$)

[ROBOT0-VT]

Yes, but then the password would be revealed in the UI, defeating the purpose of the password type. As a streamer myself, I'd very much rather avoid that.

[rrooggiieerr]

Ah, didn't know that. Better not to validate then

[ROBOT0-VT]

Hmm, I just realised bashio has a way to modify the users' options from the script.

This means that we could add the boolean options as new ones; delete the old int options; and auto-convert their int options to boolean ones once, instead of on every run.

This would include a suggested pattern from here and also make type handling for these configs more consistent (rather than having to hop between bashio::config and bashio::var)

The rebase is going to be a nightmare though 💀

Will pick this up again tomorrow, expect a lovely force-push soon =D

[rrooggiieerr]

Sorry for the misunderstanding, I actually proposed two solutions for the debugging, modify it to true/false or keep it numeric so other log levels could be added in the future. Either way, the input could be validated

[ROBOT0-VT]

The debug option is already validated, between 0 and 2. If that's not working, it's HASS's problem, not mine =D

The plan is to make logging more granular, as most of bashio's functions already log at their trace level, and there can be a lot more variance in what we tell users (and contributors).

[rrooggiieerr]

Nope, validation is not working. And actually channel number is actually also not working, also allows values outside the given range

[rrooggiieerr]

Sorry, validation is working. The number selector in the interface just allows me to select a number outside the range. I'm guessing I'm expecting to much, even though it should be possible to set a range on a number selector

[ROBOT0-VT]

Let's see if anyone ever gets around to raising it with HASS 😂 Their string length validation is also borked, hence my use of match instead of str for the SSID.

[mattlongman]

I really appreciate the effort you've both put in on this. It's not something I've been able to focus on like this for a long while!

@ROBOT0-VT I've scanned through the changes and looks good (caveat 1: I've not tested, caveat 2: I'm not familiar with bashio). I've added you and @rrooggiieerr as maintainers but obviously don't have to accept. If there are no more changes for this one, please merge (or I can).

[ROBOT0-VT]

Thanks @mattlongman ! I've still got some changes to make, and I want to clean up/rebase this branch, but then will do!

[ROBOT0-VT]

Alright, everything has been pushed and cleaned up (I hate that the PR keeps track of my rebasing 😬)

Feel free to test, either of you can test and merge if you're happy

[ROBOT0-VT]

Hiyo @rrooggiieerr @mattlongman I've been away for a bit. If neither of you get around to regression testing the major features by tomorrow, then I'll go ahead and test and merge tomorrow 👍

[mattlongman]

Hiyo @rrooggiieerr @mattlongman I've been away for a bit. If neither of you get around to regression testing the major features by tomorrow, then I'll go ahead and test and merge tomorrow 👍

Sorry, not had time to find spare hardware to test yet.