Add domain paramteter to wifi. #16
Conversation
- To be able to do OTA updates on networks that doesn't use .local as local domain parameter domain is added to wifi section. It's currently only used for OTA.
esphomeyaml/components/wifi.py
Outdated
| @@ -42,6 +42,7 @@ def validate_password(value): | |||
| vol.Optional(CONF_MANUAL_IP): AP_MANUAL_IP_SCHEMA, | |||
| }), | |||
| vol.Optional(CONF_HOSTNAME): cv.hostname, | |||
| vol.Optional(CONF_DOMAIN): cv.string, | |||
There was a problem hiding this comment.
Shouldn't we include the . here? I think it would make the purpose of this variable a bit clearer.
Also, please set the default value here, and not in config[CONF_WIFI].get(CONF_DOMAIN, 'local').
vol.Optional(CONF_DOMAIN, default="local"): cv.string,
# later:
domain = '.' + config[CONF_WIFI][CONF_DOMAIN](It helps keeping all defaults in a central place, if they're in all sorts of places it's hard to keep track of them)
There was a problem hiding this comment.
Should I make it to work with and without the dot? So it accepts both
domain: '.local' and domain: 'local' and both renders to name.local?
There was a problem hiding this comment.
I'm not really that big of a fan of these configuration options that do not strictly enforce values and try to auto-correct stuff. More of than not they end up confusing people and lead to a poor user experience.
A validator that checks if the strings starts with . and creates a vol.Invalid exception if it's invalid, would probably be better.
esphomeyaml/__main__.py
Outdated
| @@ -163,12 +163,13 @@ def upload_program(config, args, port): | |||
| _LOGGER.error("No serial port found and OTA not enabled. Can't upload!") | |||
| return -1 | |||
|
|
|||
| DOMAIN = '.' + config[CONF_WIFI].get(CONF_DOMAIN, 'local') | |||
There was a problem hiding this comment.
I think the python standard is to only have global constants with uppercase names.
(pylint isn't complaining about this because I shut the invalid-name lint off, because it produced many false positives with some recent updates to the C++ generation engine I did)
local domain parameter domain is added to wifi section. It's currently
only used for OTA.