Update Vivotek component stream source#27941
Conversation
Fix building stream URL
Make stream path optionally configurable.
c994aad to
1f05674
Compare
| "rtsp://%s:%s@%s:554/%s" | ||
| % ( | ||
| config[CONF_USERNAME], | ||
| config[CONF_PASSWORD], | ||
| config[CONF_IP_ADDRESS], | ||
| config[CONF_STREAM_PATH], | ||
| ) |
There was a problem hiding this comment.
Why not actually use f-strings here?
There was a problem hiding this comment.
I am pretty new to Python so I was not aware of f-strings. I just read through https://realpython.com/python-f-strings/ and they look pretty cool! The article did say they were introduced in Python 3.6 so as long as we don't need this code to work in < 3.6 I am happy to make the change.
There was a problem hiding this comment.
We dropped support for python <3.6, so f-strings are fine.
Use f-string to build stream source URL. This improve readability and I hear it runs faster.
| vol.Optional(CONF_SSL, default=False): cv.boolean, | ||
| vol.Optional(CONF_VERIFY_SSL, default=True): cv.boolean, | ||
| vol.Optional(CONF_FRAMERATE, default=2): cv.positive_int, | ||
| vol.Optional(CONF_STREAM_PATH, default=DEFAULT_STREAM_SOURCE): cv.string, |
There was a problem hiding this comment.
Is it impossible to detect this automatically?
There was a problem hiding this comment.
This allows choosing a different stream path mostly to allow using a different encoding. I don't see a clear standard for this at the moment so that's why it's open-ended here.
Description:
stream_pathconfiguration optionRelated issue (if applicable): fixes #26942
Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#10899
Checklist:
tox. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
python3 -m script.hassfest.requirements_all.txtby runningpython3 -m script.gen_requirements_all..coveragerc.