Use sane defaults for zwave #1891
Conversation
| DEFAULT_ZWAVE_CONFIG_PATH = os.path.join(os.path.dirname( | ||
| libopenzwave.__file__), 'config') | ||
| except NameError: | ||
| DEFAULT_ZWAVE_CONFIG_PATH = os.path.join(sys.prefix, 'share', |
There was a problem hiding this comment.
No need to default anything as without openzwave it's not going to work.
Should we detect if openzwave doesn't exist in the setup and link to instructions?
There was a problem hiding this comment.
The idea is to default to libopenzwave.__file__ as the config location if one is not specified. This will be the correct location except in edge cases where the user has modified/updated the libopenzwave config manually. The goal is for elimiating config_path: as a required yaml entry except for those rare edge cases.
You're right about the failure mode however, if libopenzwave is not found, we should exit the component then and there.
danieljkemp
force-pushed
the
zwave-config-defaults
branch
from
fb20e1d to
ad64016
Compare
|
Updated PR, fixed the clobbering of thermostat command classes, left write access checking to libopenzwave, and changed where imports are being done. |
|
Looks good to me |
|
👍 |
| @@ -175,10 +172,14 @@ def setup(hass, config): | |||
| # pylint: disable=global-statement, import-error | |||
| global NETWORK | |||
|
|
|||
| import libopenzwave | |||
There was a problem hiding this comment.
Since we can't install this using REQUIREMENTS, I would suggest we wrap this and print a proper warning to the user:
try:
import libopenzwave
except ImportError:
_LOGGER.error("You are missing required dependency Python Open Z-Wave. Please follow instructions at X")
return False
|
Looks good 🐬 Added 1 comment that would improve the experience a bit more. |
Change the zwave config default path to be valid in all but rare edge cases. This will let that config value be truely optional. Also check that libopenzwave is installed and print a warrning if it cannot be found pointing at the site.
danieljkemp
force-pushed
the
zwave-config-defaults
branch
from
a2aa5be to
c04797a
Compare
|
💃 Looking good 🐬 |
Description:
Use sane default if libopenzwave is installed. In most cases this will mean that the zwave config path will not need to e manually specified.
Verify write access to the usb device before opening. If write access is missing, print an error indicating the current user, and path of usb device.
Related issue (if applicable): #1884, verifies home-assistant/home-assistant.io#414 with readable error message.
Example entry for
configuration.yaml(if applicable):Checklist:
If code communicates with devices:
toxrun successfully. Your PR cannot be merged unless tests passREQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.If the code does not interact with devices:
toxrun successfully. Your PR cannot be merged unless tests passUse sane default if libopenzwave is installed.
Verify write access to the usb device before opening.