Skip to content

Conversation

@shroffk
Copy link
Member

@shroffk shroffk commented Mar 12, 2024

Work on making the annunciator plugable

@shroffk shroffk marked this pull request as ready for review March 18, 2024 18:18
@shroffk
Copy link
Member Author

shroffk commented Apr 5, 2024

@tynanford can you try the updated version

@tynanford
Copy link
Contributor

tynanford commented Apr 10, 2024

@shroffk Yes had a chance to test today, looking good! When I build with app-alarm-audio-annunciator in pom.xml I can confirm the timeout works for me, different alarm levels play different sounds, annunicator mute/test work fine, nag period works as expected

When I just build from this branch though, the product pom.xml contains both app-alarm-freetts-annunciator and app-alarm-audio-annunciator. And this produces behavior where both kevin16 and the audio clip are playing at the same time. How do we handle this with other pluggable systems in phoebus? I think it could confuse new users who just clone from github and build phoebus

@shroffk
Copy link
Member Author

shroffk commented Apr 11, 2024

@tynanford you are right about the duplicate annunciators.
I removed the audio one and kept the freetts one as an optional dependency of the common product.

People using the common product will get the freetts one... but site specific products will have the ability to explicitly include the annunciator they want to use... in the ALS-U case you can use the audio one. Others will have to include the freetts one.

@tynanford
Copy link
Contributor

That makes sense to me, looks good. Added Han as a reviewer as well

@tynanford
Copy link
Contributor

well looks like I can't add Han as a "reviewer" so @jeonghanlee , any issues before it is merged?

@shroffk
Copy link
Member Author

shroffk commented Apr 12, 2024

Sorry, I have fixed the SPI contributions being constructed each time.

@shroffk
Copy link
Member Author

shroffk commented Apr 17, 2024

I am ready to merge this... at the monthly meeting I mentioned how after this PR is merged site specific products will have to explicitly include the annunciator they want to use.

@shroffk shroffk merged commit 7982c09 into master Apr 17, 2024
@shroffk shroffk deleted the 2968-annunciator branch July 23, 2024 19:02
@jeonghanlee
Copy link
Contributor

jeonghanlee commented Jan 20, 2025

Just for the reference, BBC released many sound samples as a free usage for not commercial usages.

https://sound-effects.bbcrewind.co.uk/search?q=alarm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants