Skip to content

Move FFmpeg 'run_test' configuration from platform to components#17769

Closed
jjlawren wants to merge 4 commits intohome-assistant:devfrom
jjlawren:ffmpeg_allow_skip_test
Closed

Move FFmpeg 'run_test' configuration from platform to components#17769
jjlawren wants to merge 4 commits intohome-assistant:devfrom
jjlawren:ffmpeg_allow_skip_test

Conversation

@jjlawren
Copy link
Copy Markdown
Contributor

Description:

Related issue (if applicable): fixes #17495

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#7064

Example entry for configuration.yaml (if applicable):

camera:
  - platform: ffmpeg
    name: My Camera
    input: -rtsp_transport tcp -i rtsp://10.2.3.4:8554/unicast
    run_test: false

binary_sensor:
  - platform: ffmpeg_noise
    name: My Camera Sound
    input: rtsp://10.2.3.4:8554/unicast
    extra_arguments: -filter:a highpass=f=300,lowpass=f=2500,volume=volume=2 -codec:a libmp3lame -ar 16000
    initial_state: false
    duration: 2
    reset: 60
    peak: -32
    run_test: false

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@jjlawren jjlawren closed this Oct 24, 2018
@ghost ghost removed the in progress label Oct 24, 2018
@jjlawren jjlawren reopened this Oct 24, 2018
@ghost ghost added the in progress label Oct 24, 2018
@pvizeli
Copy link
Copy Markdown
Member

pvizeli commented Oct 26, 2018

I'm currently not sure if the current handling not better and we need to update the documentation. The Idea was that you can add ffmpeg sensors/camera and it will test that on startup, that makes the startup slower but for "normal" users it make the config save. If the user is sure that he doesn't need this, he can disable it on component global config with:

ffmpeg:
  run_test: false

Now, both work and not. If you disable it global, the platform settings don't work. That makes it a bit weird. Why are the old settings not working for you?

@jjlawren
Copy link
Copy Markdown
Contributor Author

jjlawren commented Oct 26, 2018

My specific use case is that I have multiple ffmpeg-based entities, where some inputs are always available and others which turn on/off. The ones that can turn on/off are sometimes not providing feeds when HA is started and it fails to create the entities. On the other hand, I'd like it to validate the inputs which I expect to always be available.

Perhaps the docs (in the components) could reflect that a global override can be set in the ffmpeg platform, but otherwise it will obey the per-component configs?

Copy link
Copy Markdown

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/home/linters/.local/bin/flake8", line 7, in 
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

@jjlawren
Copy link
Copy Markdown
Contributor Author

I've taken your suggestion and gone all-in on configuring tests on the individual components and removed the global run_test config at the platform level. This gives more control and I think is generally easier to understand. Updated docs to reflect this in the linked doc PR.

@jjlawren jjlawren changed the title Add 'run_test' configs to skip ffmpeg input test on startup Move FFmpeg 'run_test' configuration from platform to components Oct 30, 2018
@jjlawren
Copy link
Copy Markdown
Contributor Author

@pvizeli can you comment on the latest change? I’ll need to fix the tests but want to confirm the new direction is agreeable first.

@jjlawren
Copy link
Copy Markdown
Contributor Author

jjlawren commented Nov 3, 2018

Closing in favor of #18131.

@jjlawren jjlawren closed this Nov 3, 2018
@ghost ghost removed the in progress label Nov 3, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Feb 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ffmpeg_noise entity not created if input unavailable on startup

5 participants