From 79600f4a1f81e13714ca8211e8d0d8dc6d0f9efb Mon Sep 17 00:00:00 2001 From: mark Date: Mon, 11 May 2026 10:50:24 +0100 Subject: [PATCH 1/8] Refactor: remove isAvailable(), make build() return Skill<*>? Eliminates the race condition where a skill could be deemed available under one locale but then built/executed under a different locale after a language change. By merging the availability check into build() and returning null when the skill cannot be constructed, there is no longer a window between checking availability and building the skill where state can change. - Remove SkillInfo.isAvailable() abstract method - Change SkillInfo.build() return type from Skill<*> to Skill<*>? - Update all 15 SkillInfo implementations to return null if unavailable - Update SkillHandler to use mapNotNull for building skills - Update SkillSettingsScreen to use build() != null - Update test mocks and preview providers Addresses the root cause of the NPE in #412 at the architecture level. --- .../org/stypox/dicio/eval/SkillHandler.kt | 10 +++++----- .../dicio/settings/SkillSettingsScreen.kt | 2 +- .../dicio/skills/calculator/CalculatorInfo.kt | 11 ++++------- .../skills/current_time/CurrentTimeInfo.kt | 9 +++------ .../skills/fallback/text/TextFallbackInfo.kt | 4 ---- .../dicio/skills/flashlight/FlashlightInfo.kt | 14 ++++++-------- .../org/stypox/dicio/skills/joke/JokeInfo.kt | 12 +++++------- .../org/stypox/dicio/skills/joke/JokeSkill.kt | 4 ++-- .../dicio/skills/listening/ListeningInfo.kt | 9 +++------ .../stypox/dicio/skills/lyrics/LyricsInfo.kt | 9 +++------ .../org/stypox/dicio/skills/media/MediaInfo.kt | 9 +++------ .../dicio/skills/navigation/NavigationInfo.kt | 9 +++------ .../stypox/dicio/skills/notify/NotifyInfo.kt | 9 +++------ .../org/stypox/dicio/skills/open/OpenInfo.kt | 9 +++------ .../stypox/dicio/skills/search/SearchInfo.kt | 9 +++------ .../dicio/skills/telephone/TelephoneInfo.kt | 9 +++------ .../org/stypox/dicio/skills/timer/TimerInfo.kt | 11 ++++------- .../dicio/skills/translation/TranslationInfo.kt | 12 +++++------- .../stypox/dicio/skills/weather/WeatherInfo.kt | 9 +++------ .../dicio/ui/util/PreviewParameterProviders.kt | 3 +-- app/src/test/kotlin/org/stypox/dicio/Mocks.kt | 1 - .../java/org/dicio/skill/skill/SkillInfo.kt | 17 ++++++----------- skill/src/test/java/org/dicio/skill/Mocks.kt | 1 - 23 files changed, 69 insertions(+), 123 deletions(-) diff --git a/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt b/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt index 5c7c7f885..d71a23ed2 100644 --- a/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt +++ b/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt @@ -73,7 +73,7 @@ class SkillHandler @Inject constructor( private val _skillRanker = MutableStateFlow( // an initial dummy value, will be overwritten directly by the launched job - SkillRanker(listOf(), buildSkillFromInfo(fallbackSkillInfoList[0])) + SkillRanker(listOf(), buildSkillFromInfo(fallbackSkillInfoList[0])!!) ) val skillRanker: StateFlow = _skillRanker @@ -87,18 +87,18 @@ class SkillHandler @Inject constructor( val newEnabledSkillsInfo = allSkillInfoList .filter { enabledSkills.getOrDefault(it.id, true) } - .filter { it.isAvailable(skillContext) } + .filter { it.build(skillContext) != null } _enabledSkillsInfo.value = newEnabledSkillsInfo _skillRanker.value = SkillRanker( - newEnabledSkillsInfo.map(::buildSkillFromInfo), - buildSkillFromInfo(fallbackSkillInfoList[0]), + newEnabledSkillsInfo.mapNotNull(::buildSkillFromInfo), + buildSkillFromInfo(fallbackSkillInfoList[0])!!, ) } } } - private fun buildSkillFromInfo(skillInfo: SkillInfo): Skill<*> { + private fun buildSkillFromInfo(skillInfo: SkillInfo): Skill<*>? { return skillInfo.build(skillContext) } diff --git a/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsScreen.kt b/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsScreen.kt index 08e7e4945..0e41ef9f0 100644 --- a/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsScreen.kt +++ b/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsScreen.kt @@ -115,7 +115,7 @@ fun SkillSettingsScreen( items(skills) { skill -> SkillSettingsItem( skill = skill, - isAvailable = skill.isAvailable(viewModel.skillContext), + isAvailable = skill.build(viewModel.skillContext) != null, enabled = enabledSkills.getOrDefault(skill.id, true), setEnabled = { enabled -> viewModel.setSkillEnabled(skill.id, enabled) } ) diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/calculator/CalculatorInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/calculator/CalculatorInfo.kt index fd266df98..b7b385199 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/calculator/CalculatorInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/calculator/CalculatorInfo.kt @@ -22,13 +22,10 @@ object CalculatorInfo : SkillInfo("calculator") { override fun icon() = rememberVectorPainter(Icons.Default.Calculate) - override fun isAvailable(ctx: SkillContext): Boolean { - return Sentences.Calculator[ctx.sentencesLanguage] != null && - Sentences.CalculatorOperators[ctx.sentencesLanguage] != null && - ctx.parserFormatter != null - } - - override fun build(ctx: SkillContext): Skill<*> { + override fun build(ctx: SkillContext): Skill<*>? { + if (Sentences.Calculator[ctx.sentencesLanguage] == null || + Sentences.CalculatorOperators[ctx.sentencesLanguage] == null || + ctx.parserFormatter == null) return null return CalculatorSkill(CalculatorInfo, Sentences.Calculator[ctx.sentencesLanguage]!!) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/current_time/CurrentTimeInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/current_time/CurrentTimeInfo.kt index 8efa0b330..d32a47e63 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/current_time/CurrentTimeInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/current_time/CurrentTimeInfo.kt @@ -22,11 +22,8 @@ object CurrentTimeInfo : SkillInfo("current_time") { override fun icon() = rememberVectorPainter(Icons.Default.Watch) - override fun isAvailable(ctx: SkillContext): Boolean { - return Sentences.CurrentTime[ctx.sentencesLanguage] != null - } - - override fun build(ctx: SkillContext): Skill<*> { - return CurrentTimeSkill(CurrentTimeInfo, Sentences.CurrentTime[ctx.sentencesLanguage]!!) + override fun build(ctx: SkillContext): Skill<*>? { + val data = Sentences.CurrentTime[ctx.sentencesLanguage] ?: return null + return CurrentTimeSkill(CurrentTimeInfo, data) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/fallback/text/TextFallbackInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/fallback/text/TextFallbackInfo.kt index bbb0f353b..b17e406ad 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/fallback/text/TextFallbackInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/fallback/text/TextFallbackInfo.kt @@ -21,10 +21,6 @@ object TextFallbackInfo : SkillInfo("text") { override fun icon() = rememberVectorPainter(Icons.Default.Warning) - override fun isAvailable(ctx: SkillContext): Boolean { - return true - } - override fun build(ctx: SkillContext): Skill<*> { return TextFallbackSkill(TextFallbackInfo) } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/flashlight/FlashlightInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/flashlight/FlashlightInfo.kt index 0bf9fe525..cf73662c1 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/flashlight/FlashlightInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/flashlight/FlashlightInfo.kt @@ -24,13 +24,11 @@ object FlashlightInfo : SkillInfo("flashlight") { override fun icon() = rememberVectorPainter(Icons.Default.FlashlightOn) - override fun isAvailable(ctx: SkillContext): Boolean { - return Sentences.Flashlight[ctx.sentencesLanguage] != null && - ctx.android.packageManager.hasSystemFeature(PackageManager.FEATURE_CAMERA_FLASH) - } - - @SuppressLint("NewApi") // since build() is not called if isAvailable() returned false - override fun build(ctx: SkillContext): Skill<*> { - return FlashlightSkill(FlashlightInfo, Sentences.Flashlight[ctx.sentencesLanguage]!!) + @SuppressLint("NewApi") + override fun build(ctx: SkillContext): Skill<*>? { + val data = Sentences.Flashlight[ctx.sentencesLanguage] ?: return null + if (!ctx.android.packageManager.hasSystemFeature(PackageManager.FEATURE_CAMERA_FLASH)) + return null + return FlashlightSkill(FlashlightInfo, data) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/joke/JokeInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/joke/JokeInfo.kt index 4b369a9bd..4e76ab408 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/joke/JokeInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/joke/JokeInfo.kt @@ -23,12 +23,10 @@ object JokeInfo : SkillInfo("Joke") { override fun icon() = rememberVectorPainter(Icons.Default.EmojiEmotions) - override fun isAvailable(ctx: SkillContext): Boolean { - return (Sentences.Joke[ctx.sentencesLanguage] != null) && - LocaleUtils.isLocaleSupported(ctx.locale, JokeSkill.JOKE_SUPPORTED_LOCALES) - } - - override fun build(ctx: SkillContext): Skill<*> { - return JokeSkill(JokeInfo, Sentences.Joke[ctx.sentencesLanguage]!!) + override fun build(ctx: SkillContext): Skill<*>? { + val data = Sentences.Joke[ctx.sentencesLanguage] ?: return null + if (!LocaleUtils.isLocaleSupported(ctx.locale, JokeSkill.JOKE_SUPPORTED_LOCALES)) + return null + return JokeSkill(JokeInfo, data) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/joke/JokeSkill.kt b/app/src/main/kotlin/org/stypox/dicio/skills/joke/JokeSkill.kt index 9675244b6..f09282a5c 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/joke/JokeSkill.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/joke/JokeSkill.kt @@ -13,8 +13,8 @@ import org.stypox.dicio.util.LocaleUtils class JokeSkill(correspondingSkillInfo: SkillInfo, data: StandardRecognizerData) : StandardRecognizerSkill(correspondingSkillInfo, data) { override suspend fun generateOutput(ctx: SkillContext, inputData: Joke): SkillOutput { - // we can use !! because the JokeInfo would have declared this skill unavailable - // if the current locale was not among the supported ones + // safe to use !! because build() returns null if the locale is not supported, + // so this skill instance can only exist when the locale is in JOKE_SUPPORTED_LOCALES val locale = LocaleUtils.resolveSupportedLocale(ctx.locale, JOKE_SUPPORTED_LOCALES)!! if (locale == "en") { diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/listening/ListeningInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/listening/ListeningInfo.kt index 139ba594c..aeecbedde 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/listening/ListeningInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/listening/ListeningInfo.kt @@ -26,11 +26,8 @@ class ListeningInfo( override fun icon() = rememberVectorPainter(Icons.Default.Hearing) - override fun isAvailable(ctx: SkillContext): Boolean { - return Sentences.Listening[ctx.sentencesLanguage] != null - } - - override fun build(ctx: SkillContext): Skill<*> { - return ListeningSkill(this, Sentences.Listening[ctx.sentencesLanguage]!!) + override fun build(ctx: SkillContext): Skill<*>? { + val data = Sentences.Listening[ctx.sentencesLanguage] ?: return null + return ListeningSkill(this, data) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/lyrics/LyricsInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/lyrics/LyricsInfo.kt index 6f9240cc8..ac43053f7 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/lyrics/LyricsInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/lyrics/LyricsInfo.kt @@ -22,11 +22,8 @@ object LyricsInfo : SkillInfo("lyrics") { override fun icon() = rememberVectorPainter(Icons.Default.MusicNote) - override fun isAvailable(ctx: SkillContext): Boolean { - return Sentences.Lyrics[ctx.sentencesLanguage] != null - } - - override fun build(ctx: SkillContext): Skill<*> { - return LyricsSkill(LyricsInfo, Sentences.Lyrics[ctx.sentencesLanguage]!!) + override fun build(ctx: SkillContext): Skill<*>? { + val data = Sentences.Lyrics[ctx.sentencesLanguage] ?: return null + return LyricsSkill(LyricsInfo, data) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/media/MediaInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/media/MediaInfo.kt index 8558fbba5..da250241c 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/media/MediaInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/media/MediaInfo.kt @@ -22,11 +22,8 @@ object MediaInfo : SkillInfo("media") { override fun icon() = rememberVectorPainter(Icons.AutoMirrored.Filled.QueueMusic) - override fun isAvailable(ctx: SkillContext): Boolean { - return Sentences.Media[ctx.sentencesLanguage] != null - } - - override fun build(ctx: SkillContext): Skill<*> { - return MediaSkill(MediaInfo, Sentences.Media[ctx.sentencesLanguage]!!) + override fun build(ctx: SkillContext): Skill<*>? { + val data = Sentences.Media[ctx.sentencesLanguage] ?: return null + return MediaSkill(MediaInfo, data) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/navigation/NavigationInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/navigation/NavigationInfo.kt index 4c1f45b75..fef9f538d 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/navigation/NavigationInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/navigation/NavigationInfo.kt @@ -25,11 +25,8 @@ object NavigationInfo : SkillInfo("navigation") { override fun icon() = rememberVectorPainter(Icons.Default.Directions) - override fun isAvailable(ctx: SkillContext): Boolean { - return Sentences.Navigation[ctx.sentencesLanguage] != null - } - - override fun build(ctx: SkillContext): Skill<*> { - return NavigationSkill(NavigationInfo, Sentences.Navigation[ctx.sentencesLanguage]!!) + override fun build(ctx: SkillContext): Skill<*>? { + val data = Sentences.Navigation[ctx.sentencesLanguage] ?: return null + return NavigationSkill(NavigationInfo, data) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/notify/NotifyInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/notify/NotifyInfo.kt index 19918d2aa..c86bfc6bb 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/notify/NotifyInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/notify/NotifyInfo.kt @@ -24,14 +24,11 @@ object NotifyInfo: SkillInfo("notify") { override fun icon() = rememberVectorPainter(Icons.Default.NotificationsActive) - override fun isAvailable(ctx: SkillContext): Boolean { - return Sentences.Notify[ctx.sentencesLanguage] != null - } - override val neededPermissions: List = listOf(PERMISSION_NOTIFICATION_LISTENER) - override fun build(ctx: SkillContext): Skill<*> { - return NotifySkill(NotifyInfo, Sentences.Notify[ctx.sentencesLanguage]!!) + override fun build(ctx: SkillContext): Skill<*>? { + val data = Sentences.Notify[ctx.sentencesLanguage] ?: return null + return NotifySkill(NotifyInfo, data) } } \ No newline at end of file diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/open/OpenInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/open/OpenInfo.kt index adc799b29..913159cdf 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/open/OpenInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/open/OpenInfo.kt @@ -25,11 +25,8 @@ object OpenInfo : SkillInfo("open") { override fun icon() = rememberVectorPainter(Icons.AutoMirrored.Filled.OpenInNew) - override fun isAvailable(ctx: SkillContext): Boolean { - return Sentences.Open[ctx.sentencesLanguage] != null - } - - override fun build(ctx: SkillContext): Skill<*> { - return OpenSkill(OpenInfo, Sentences.Open[ctx.sentencesLanguage]!!) + override fun build(ctx: SkillContext): Skill<*>? { + val data = Sentences.Open[ctx.sentencesLanguage] ?: return null + return OpenSkill(OpenInfo, data) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/search/SearchInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/search/SearchInfo.kt index 0dbd0dbc4..1a4448225 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/search/SearchInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/search/SearchInfo.kt @@ -22,11 +22,8 @@ object SearchInfo : SkillInfo("search") { override fun icon() = rememberVectorPainter(Icons.Default.Search) - override fun isAvailable(ctx: SkillContext): Boolean { - return Sentences.Search[ctx.sentencesLanguage] != null - } - - override fun build(ctx: SkillContext): Skill<*> { - return SearchSkill(SearchInfo, Sentences.Search[ctx.sentencesLanguage]!!) + override fun build(ctx: SkillContext): Skill<*>? { + val data = Sentences.Search[ctx.sentencesLanguage] ?: return null + return SearchSkill(SearchInfo, data) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneInfo.kt index c14556dfb..e80ada728 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneInfo.kt @@ -31,12 +31,9 @@ object TelephoneInfo : SkillInfo("telephone") { override val neededPermissions: List = listOf(PERMISSION_READ_CONTACTS, PERMISSION_CALL_PHONE) - override fun isAvailable(ctx: SkillContext): Boolean { - return Sentences.Telephone[ctx.sentencesLanguage] != null && - Sentences.UtilYesNo[ctx.sentencesLanguage] != null - } - - override fun build(ctx: SkillContext): Skill<*> { + override fun build(ctx: SkillContext): Skill<*>? { + if (Sentences.Telephone[ctx.sentencesLanguage] == null || + Sentences.UtilYesNo[ctx.sentencesLanguage] == null) return null return TelephoneSkill(TelephoneInfo, Sentences.Telephone[ctx.sentencesLanguage]!!) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerInfo.kt index a838dade8..98da23a95 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerInfo.kt @@ -24,13 +24,10 @@ object TimerInfo : SkillInfo("timer") { override fun icon() = rememberVectorPainter(Icons.Default.Timer) - override fun isAvailable(ctx: SkillContext): Boolean { - return Sentences.Timer[ctx.sentencesLanguage] != null - && Sentences.UtilYesNo[ctx.sentencesLanguage] != null - && ctx.parserFormatter != null - } - - override fun build(ctx: SkillContext): Skill<*> { + override fun build(ctx: SkillContext): Skill<*>? { + if (Sentences.Timer[ctx.sentencesLanguage] == null || + Sentences.UtilYesNo[ctx.sentencesLanguage] == null || + ctx.parserFormatter == null) return null return TimerSkill(TimerInfo, Sentences.Timer[ctx.sentencesLanguage]!!) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/translation/TranslationInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/translation/TranslationInfo.kt index 42654317a..39cb6e652 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/translation/TranslationInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/translation/TranslationInfo.kt @@ -23,12 +23,10 @@ object TranslationInfo : SkillInfo("translation") { override fun icon() = rememberVectorPainter(Icons.Default.Language) - override fun isAvailable(ctx: SkillContext): Boolean { - return (Sentences.Translation[ctx.sentencesLanguage] != null) && - LocaleUtils.isLocaleSupported(ctx.locale, TranslationSkill.TRANSLATE_SUPPORTED_LOCALES) - } - - override fun build(ctx: SkillContext): Skill<*> { - return TranslationSkill(TranslationInfo, Sentences.Translation[ctx.sentencesLanguage]!!) + override fun build(ctx: SkillContext): Skill<*>? { + val data = Sentences.Translation[ctx.sentencesLanguage] ?: return null + if (!LocaleUtils.isLocaleSupported(ctx.locale, TranslationSkill.TRANSLATE_SUPPORTED_LOCALES)) + return null + return TranslationSkill(TranslationInfo, data) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/weather/WeatherInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/weather/WeatherInfo.kt index 5b7a4366e..866c446a7 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/weather/WeatherInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/weather/WeatherInfo.kt @@ -35,12 +35,9 @@ object WeatherInfo : SkillInfo("weather") { override fun icon() = rememberVectorPainter(Icons.Default.Cloud) - override fun isAvailable(ctx: SkillContext): Boolean { - return Sentences.Weather[ctx.sentencesLanguage] != null - } - - override fun build(ctx: SkillContext): Skill<*> { - return WeatherSkill(WeatherInfo, Sentences.Weather[ctx.sentencesLanguage]!!) + override fun build(ctx: SkillContext): Skill<*>? { + val data = Sentences.Weather[ctx.sentencesLanguage] ?: return null + return WeatherSkill(WeatherInfo, data) } // no need to use Hilt injection here, let DataStore take care of handling the singleton itself diff --git a/app/src/main/kotlin/org/stypox/dicio/ui/util/PreviewParameterProviders.kt b/app/src/main/kotlin/org/stypox/dicio/ui/util/PreviewParameterProviders.kt index 3419ecde0..9c8e0637b 100644 --- a/app/src/main/kotlin/org/stypox/dicio/ui/util/PreviewParameterProviders.kt +++ b/app/src/main/kotlin/org/stypox/dicio/ui/util/PreviewParameterProviders.kt @@ -47,8 +47,7 @@ class SkillInfoPreviews : CollectionPreviewParameterProvider(listOf( override fun name(context: Context) = "Long name lorem ipsum dolor sit amet, consectetur" override fun sentenceExample(context: Context) = "Long sentence ".repeat(20) @Composable override fun icon() = rememberVectorPainter(Icons.Default.Extension) - override fun isAvailable(ctx: SkillContext) = true - override fun build(ctx: SkillContext) = error("not-implemented preview-only") + override fun build(ctx: SkillContext): Nothing = error("not-implemented preview-only") }, )) diff --git a/app/src/test/kotlin/org/stypox/dicio/Mocks.kt b/app/src/test/kotlin/org/stypox/dicio/Mocks.kt index b35b49a89..edec10bde 100644 --- a/app/src/test/kotlin/org/stypox/dicio/Mocks.kt +++ b/app/src/test/kotlin/org/stypox/dicio/Mocks.kt @@ -29,7 +29,6 @@ object MockSkillInfo : SkillInfo("") { override fun name(context: Context): String = mocked() override fun sentenceExample(context: Context): String = mocked() @Composable override fun icon(): Painter = mocked() - override fun isAvailable(ctx: SkillContext) = mocked() override fun build(ctx: SkillContext) = mocked() } diff --git a/skill/src/main/java/org/dicio/skill/skill/SkillInfo.kt b/skill/src/main/java/org/dicio/skill/skill/SkillInfo.kt index 57e931309..57b0022be 100644 --- a/skill/src/main/java/org/dicio/skill/skill/SkillInfo.kt +++ b/skill/src/main/java/org/dicio/skill/skill/SkillInfo.kt @@ -47,19 +47,14 @@ abstract class SkillInfo( open val neededPermissions: List = listOf() /** - * Use this method to signal that the skill is not available in case, for example, the user - * locale is not supported. + * Builds an instance of the [Skill] this [SkillInfo] object represents, or returns null if + * the skill is not available (e.g. the user locale is not supported, or required hardware is + * missing). Combining availability checking and construction into a single method eliminates + * race conditions where the context could change between an availability check and build. * @param ctx the skill context with useful resources, see [SkillContext] - * @return whether this skill can be used with the current system configuration or not + * @return a skill, or null if the skill is not available */ - abstract fun isAvailable(ctx: SkillContext): Boolean - - /** - * Builds an instance of the [Skill] this [SkillInfo] object represents. - * @param ctx the skill context with useful resources, see [SkillContext] - * @return a skill - */ - abstract fun build(ctx: SkillContext): Skill<*> + abstract fun build(ctx: SkillContext): Skill<*>? /** * Provides a settings screen for this skill, allowing the user to customize it to diff --git a/skill/src/test/java/org/dicio/skill/Mocks.kt b/skill/src/test/java/org/dicio/skill/Mocks.kt index 6c7af143c..b5d67759f 100644 --- a/skill/src/test/java/org/dicio/skill/Mocks.kt +++ b/skill/src/test/java/org/dicio/skill/Mocks.kt @@ -28,7 +28,6 @@ object MockSkillInfo : SkillInfo("") { override fun name(context: Context): String = mocked() override fun sentenceExample(context: Context): String = mocked() @Composable override fun icon(): Painter = mocked() - override fun isAvailable(ctx: SkillContext) = mocked() override fun build(ctx: SkillContext) = mocked() } From 67b93c8e7cf515eea5d43e34c69a5bbe2aabef5f Mon Sep 17 00:00:00 2001 From: Mark Retallack Date: Tue, 12 May 2026 21:37:04 +0100 Subject: [PATCH 2/8] Update app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt Co-authored-by: Stypox --- app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt b/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt index d71a23ed2..64d1ee1bf 100644 --- a/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt +++ b/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt @@ -87,7 +87,7 @@ class SkillHandler @Inject constructor( val newEnabledSkillsInfo = allSkillInfoList .filter { enabledSkills.getOrDefault(it.id, true) } - .filter { it.build(skillContext) != null } + .mapNotNull { info -> info.build(skillContext)?.let { skill -> Pair(info, skill) } } _enabledSkillsInfo.value = newEnabledSkillsInfo _skillRanker.value = SkillRanker( From 2fee5952864fa496a6bd5afec09dc10a634d9afc Mon Sep 17 00:00:00 2001 From: Mark Retallack Date: Tue, 12 May 2026 21:37:19 +0100 Subject: [PATCH 3/8] Update app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt Co-authored-by: Stypox --- app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt b/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt index 64d1ee1bf..657451187 100644 --- a/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt +++ b/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt @@ -89,7 +89,7 @@ class SkillHandler @Inject constructor( .filter { enabledSkills.getOrDefault(it.id, true) } .mapNotNull { info -> info.build(skillContext)?.let { skill -> Pair(info, skill) } } - _enabledSkillsInfo.value = newEnabledSkillsInfo + _enabledSkillsInfo.value = newEnabledSkillsInfo.map { (info, _skill) -> info } _skillRanker.value = SkillRanker( newEnabledSkillsInfo.mapNotNull(::buildSkillFromInfo), buildSkillFromInfo(fallbackSkillInfoList[0])!!, From 963bfc6e5ff2c68a613623cf4944fcdd5dbf61da Mon Sep 17 00:00:00 2001 From: Mark Retallack Date: Tue, 12 May 2026 21:37:34 +0100 Subject: [PATCH 4/8] Update app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt Co-authored-by: Stypox --- app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt b/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt index 657451187..afe282e98 100644 --- a/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt +++ b/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt @@ -91,7 +91,7 @@ class SkillHandler @Inject constructor( _enabledSkillsInfo.value = newEnabledSkillsInfo.map { (info, _skill) -> info } _skillRanker.value = SkillRanker( - newEnabledSkillsInfo.mapNotNull(::buildSkillFromInfo), + newEnabledSkillsInfo.map { (_info, skill) -> skill }, buildSkillFromInfo(fallbackSkillInfoList[0])!!, ) } From f42087a4e4dd3160629aad8f97df99e4beb4e22a Mon Sep 17 00:00:00 2001 From: mark Date: Tue, 12 May 2026 21:55:46 +0100 Subject: [PATCH 5/8] Address review feedback: eliminate remaining race conditions - Remove buildSkillFromInfo helper, inline fallback build - CalculatorSkill: accept operators as constructor param, no runtime !! - JokeSkill: accept resolved locale as constructor param, no runtime !! - TelephoneInfo/TimerInfo: use ?: return null pattern, no !! - FlashlightInfo: remove @SuppressLint("NewApi") - SkillSettingsScreen: source availability from enabledSkillsInfo via view model instead of calling build() in composable --- .../kotlin/org/stypox/dicio/eval/SkillHandler.kt | 9 ++------- .../stypox/dicio/settings/SkillSettingsScreen.kt | 3 ++- .../dicio/settings/SkillSettingsViewModel.kt | 3 ++- .../dicio/skills/calculator/CalculatorInfo.kt | 8 ++++---- .../dicio/skills/calculator/CalculatorSkill.kt | 8 +++++--- .../dicio/skills/flashlight/FlashlightInfo.kt | 2 -- .../org/stypox/dicio/skills/joke/JokeInfo.kt | 6 +++--- .../org/stypox/dicio/skills/joke/JokeSkill.kt | 16 +++++++--------- .../dicio/skills/telephone/TelephoneInfo.kt | 6 +++--- .../org/stypox/dicio/skills/timer/TimerInfo.kt | 8 ++++---- 10 files changed, 32 insertions(+), 37 deletions(-) diff --git a/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt b/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt index afe282e98..3b98f644b 100644 --- a/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt +++ b/app/src/main/kotlin/org/stypox/dicio/eval/SkillHandler.kt @@ -10,7 +10,6 @@ import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.launch -import org.dicio.skill.skill.Skill import org.dicio.skill.skill.SkillInfo import org.stypox.dicio.di.LocaleManager import org.stypox.dicio.di.SkillContextImpl @@ -73,7 +72,7 @@ class SkillHandler @Inject constructor( private val _skillRanker = MutableStateFlow( // an initial dummy value, will be overwritten directly by the launched job - SkillRanker(listOf(), buildSkillFromInfo(fallbackSkillInfoList[0])!!) + SkillRanker(listOf(), fallbackSkillInfoList[0].build(skillContext)!!) ) val skillRanker: StateFlow = _skillRanker @@ -92,16 +91,12 @@ class SkillHandler @Inject constructor( _enabledSkillsInfo.value = newEnabledSkillsInfo.map { (info, _skill) -> info } _skillRanker.value = SkillRanker( newEnabledSkillsInfo.map { (_info, skill) -> skill }, - buildSkillFromInfo(fallbackSkillInfoList[0])!!, + fallbackSkillInfoList[0].build(skillContext)!!, ) } } } - private fun buildSkillFromInfo(skillInfo: SkillInfo): Skill<*>? { - return skillInfo.build(skillContext) - } - companion object { fun newForPreviews(context: Context): SkillHandler { return SkillHandler( diff --git a/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsScreen.kt b/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsScreen.kt index 0e41ef9f0..3bc018823 100644 --- a/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsScreen.kt +++ b/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsScreen.kt @@ -89,6 +89,7 @@ fun SkillSettingsScreen( ) { val skills = viewModel.skills val enabledSkills by viewModel.enabledSkills.collectAsState() + val enabledSkillsInfo by viewModel.enabledSkillsInfo.collectAsState() LazyColumn( contentPadding = PaddingValues(top = 4.dp, bottom = 4.dp), @@ -115,7 +116,7 @@ fun SkillSettingsScreen( items(skills) { skill -> SkillSettingsItem( skill = skill, - isAvailable = skill.build(viewModel.skillContext) != null, + isAvailable = enabledSkillsInfo?.contains(skill) != false, enabled = enabledSkills.getOrDefault(skill.id, true), setEnabled = { enabled -> viewModel.setSkillEnabled(skill.id, enabled) } ) diff --git a/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsViewModel.kt b/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsViewModel.kt index 234840520..1f5c2d853 100644 --- a/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsViewModel.kt +++ b/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsViewModel.kt @@ -7,7 +7,6 @@ import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch -import org.dicio.skill.context.SkillContext import org.dicio.skill.skill.SkillInfo import org.stypox.dicio.di.SkillContextInternal import org.stypox.dicio.settings.datastore.UserSettings @@ -26,6 +25,8 @@ class SkillSettingsViewModel @Inject constructor( val skills: List get() = skillHandler.allSkillInfoList + val enabledSkillsInfo = skillHandler.enabledSkillsInfo + // run blocking because the settings screen cannot start if settings have not been loaded yet val enabledSkills = dataStore.data .map { it.enabledSkillsMap } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/calculator/CalculatorInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/calculator/CalculatorInfo.kt index b7b385199..2abaa4fd8 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/calculator/CalculatorInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/calculator/CalculatorInfo.kt @@ -23,9 +23,9 @@ object CalculatorInfo : SkillInfo("calculator") { rememberVectorPainter(Icons.Default.Calculate) override fun build(ctx: SkillContext): Skill<*>? { - if (Sentences.Calculator[ctx.sentencesLanguage] == null || - Sentences.CalculatorOperators[ctx.sentencesLanguage] == null || - ctx.parserFormatter == null) return null - return CalculatorSkill(CalculatorInfo, Sentences.Calculator[ctx.sentencesLanguage]!!) + val sentences = Sentences.Calculator[ctx.sentencesLanguage] ?: return null + val operators = Sentences.CalculatorOperators[ctx.sentencesLanguage] ?: return null + if (ctx.parserFormatter == null) return null + return CalculatorSkill(CalculatorInfo, sentences, operators) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/calculator/CalculatorSkill.kt b/app/src/main/kotlin/org/stypox/dicio/skills/calculator/CalculatorSkill.kt index c2927d1c3..32f5cd67f 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/calculator/CalculatorSkill.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/calculator/CalculatorSkill.kt @@ -12,8 +12,11 @@ import org.stypox.dicio.sentences.Sentences.CalculatorOperators import java.text.DecimalFormat import java.text.DecimalFormatSymbols -class CalculatorSkill(correspondingSkillInfo: SkillInfo, data: StandardRecognizerData) - : StandardRecognizerSkill(correspondingSkillInfo, data) { +class CalculatorSkill( + correspondingSkillInfo: SkillInfo, + data: StandardRecognizerData, + private val operatorRecognizerData: StandardRecognizerData, +) : StandardRecognizerSkill(correspondingSkillInfo, data) { private fun getOperation( ctx: SkillContext, @@ -45,7 +48,6 @@ class CalculatorSkill(correspondingSkillInfo: SkillInfo, data: StandardRecognize return CalculatorOutput(null, "", "") } - val operatorRecognizerData = CalculatorOperators[ctx.sentencesLanguage]!! var firstNumber: Number var i: Int if (textWithNumbers[0] is Number) { diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/flashlight/FlashlightInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/flashlight/FlashlightInfo.kt index cf73662c1..217abc037 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/flashlight/FlashlightInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/flashlight/FlashlightInfo.kt @@ -1,6 +1,5 @@ package org.stypox.dicio.skills.flashlight -import android.annotation.SuppressLint import android.content.Context import android.content.pm.PackageManager import androidx.compose.material.icons.Icons @@ -24,7 +23,6 @@ object FlashlightInfo : SkillInfo("flashlight") { override fun icon() = rememberVectorPainter(Icons.Default.FlashlightOn) - @SuppressLint("NewApi") override fun build(ctx: SkillContext): Skill<*>? { val data = Sentences.Flashlight[ctx.sentencesLanguage] ?: return null if (!ctx.android.packageManager.hasSystemFeature(PackageManager.FEATURE_CAMERA_FLASH)) diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/joke/JokeInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/joke/JokeInfo.kt index 4e76ab408..8d728e042 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/joke/JokeInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/joke/JokeInfo.kt @@ -25,8 +25,8 @@ object JokeInfo : SkillInfo("Joke") { override fun build(ctx: SkillContext): Skill<*>? { val data = Sentences.Joke[ctx.sentencesLanguage] ?: return null - if (!LocaleUtils.isLocaleSupported(ctx.locale, JokeSkill.JOKE_SUPPORTED_LOCALES)) - return null - return JokeSkill(JokeInfo, data) + val locale = LocaleUtils.resolveSupportedLocale(ctx.locale, JokeSkill.JOKE_SUPPORTED_LOCALES) + ?: return null + return JokeSkill(JokeInfo, data, locale) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/joke/JokeSkill.kt b/app/src/main/kotlin/org/stypox/dicio/skills/joke/JokeSkill.kt index f09282a5c..b410cb8d5 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/joke/JokeSkill.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/joke/JokeSkill.kt @@ -8,16 +8,14 @@ import org.dicio.skill.standard.StandardRecognizerSkill import org.json.JSONObject import org.stypox.dicio.sentences.Sentences.Joke import org.stypox.dicio.util.ConnectionUtils -import org.stypox.dicio.util.LocaleUtils -class JokeSkill(correspondingSkillInfo: SkillInfo, data: StandardRecognizerData) - : StandardRecognizerSkill(correspondingSkillInfo, data) { +class JokeSkill( + correspondingSkillInfo: SkillInfo, + data: StandardRecognizerData, + private val resolvedLocale: String, +) : StandardRecognizerSkill(correspondingSkillInfo, data) { override suspend fun generateOutput(ctx: SkillContext, inputData: Joke): SkillOutput { - // safe to use !! because build() returns null if the locale is not supported, - // so this skill instance can only exist when the locale is in JOKE_SUPPORTED_LOCALES - val locale = LocaleUtils.resolveSupportedLocale(ctx.locale, JOKE_SUPPORTED_LOCALES)!! - - if (locale == "en") { + if (resolvedLocale == "en") { val joke: JSONObject = ConnectionUtils.getPageJson(RANDOM_JOKE_URL_EN) return JokeOutput.Success( setup = joke.getString("setup"), @@ -25,7 +23,7 @@ class JokeSkill(correspondingSkillInfo: SkillInfo, data: StandardRecognizerData< ) } else { val joke: JSONObject = ConnectionUtils.getPageJson( - "$RANDOM_JOKE_URL?lang=$locale&safe-mode&type=twopart" + "$RANDOM_JOKE_URL?lang=$resolvedLocale&safe-mode&type=twopart" ) return JokeOutput.Success( setup = joke.getString("setup"), diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneInfo.kt index e80ada728..2312b6d40 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneInfo.kt @@ -32,8 +32,8 @@ object TelephoneInfo : SkillInfo("telephone") { = listOf(PERMISSION_READ_CONTACTS, PERMISSION_CALL_PHONE) override fun build(ctx: SkillContext): Skill<*>? { - if (Sentences.Telephone[ctx.sentencesLanguage] == null || - Sentences.UtilYesNo[ctx.sentencesLanguage] == null) return null - return TelephoneSkill(TelephoneInfo, Sentences.Telephone[ctx.sentencesLanguage]!!) + val data = Sentences.Telephone[ctx.sentencesLanguage] ?: return null + Sentences.UtilYesNo[ctx.sentencesLanguage] ?: return null + return TelephoneSkill(TelephoneInfo, data) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerInfo.kt index 98da23a95..9ee05cee2 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerInfo.kt @@ -25,9 +25,9 @@ object TimerInfo : SkillInfo("timer") { rememberVectorPainter(Icons.Default.Timer) override fun build(ctx: SkillContext): Skill<*>? { - if (Sentences.Timer[ctx.sentencesLanguage] == null || - Sentences.UtilYesNo[ctx.sentencesLanguage] == null || - ctx.parserFormatter == null) return null - return TimerSkill(TimerInfo, Sentences.Timer[ctx.sentencesLanguage]!!) + val data = Sentences.Timer[ctx.sentencesLanguage] ?: return null + Sentences.UtilYesNo[ctx.sentencesLanguage] ?: return null + if (ctx.parserFormatter == null) return null + return TimerSkill(TimerInfo, data) } } From 16ed5ae1de4c7ace0ad48636065eabe71ced027e Mon Sep 17 00:00:00 2001 From: mark Date: Tue, 12 May 2026 22:05:34 +0100 Subject: [PATCH 6/8] Fix: correctly show disabled-but-available skills in settings enabledSkillsInfo only contains skills that are enabled AND available. A skill disabled by the user would incorrectly appear as unavailable (greyed out). Now disabled skills are assumed available since we cannot determine availability without building them. --- .../org/stypox/dicio/settings/SkillSettingsScreen.kt | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsScreen.kt b/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsScreen.kt index 3bc018823..a06b0f12e 100644 --- a/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsScreen.kt +++ b/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsScreen.kt @@ -114,9 +114,18 @@ fun SkillSettingsScreen( } } items(skills) { skill -> + // enabledSkillsInfo contains skills that are both enabled AND available. + // A skill is "available" if: + // - it's in enabledSkillsInfo (enabled + buildable), OR + // - it's disabled by the user (we can't tell availability from the list, + // so we assume available — the worst case is a disabled skill shows as + // available, which is fine since it's toggled off anyway) + val isDisabledByUser = !enabledSkills.getOrDefault(skill.id, true) + val isAvailable = isDisabledByUser + || enabledSkillsInfo?.contains(skill) != false SkillSettingsItem( skill = skill, - isAvailable = enabledSkillsInfo?.contains(skill) != false, + isAvailable = isAvailable, enabled = enabledSkills.getOrDefault(skill.id, true), setEnabled = { enabled -> viewModel.setSkillEnabled(skill.id, enabled) } ) From 3d9373aa7adf276736d832db935760e707285bcc Mon Sep 17 00:00:00 2001 From: mark Date: Wed, 13 May 2026 16:53:26 +0100 Subject: [PATCH 7/8] Fix: restore @SuppressLint("NewApi") on FlashlightInfo.build() FlashlightSkill uses API 23+ camera APIs. The annotation is needed since minSdk is 21, but the call is guarded by the FEATURE_CAMERA_FLASH check which ensures the code only runs on capable devices. --- .../kotlin/org/stypox/dicio/skills/flashlight/FlashlightInfo.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/flashlight/FlashlightInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/flashlight/FlashlightInfo.kt index 217abc037..1c7b2ca11 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/flashlight/FlashlightInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/flashlight/FlashlightInfo.kt @@ -1,5 +1,6 @@ package org.stypox.dicio.skills.flashlight +import android.annotation.SuppressLint import android.content.Context import android.content.pm.PackageManager import androidx.compose.material.icons.Icons @@ -23,6 +24,7 @@ object FlashlightInfo : SkillInfo("flashlight") { override fun icon() = rememberVectorPainter(Icons.Default.FlashlightOn) + @SuppressLint("NewApi") // FlashlightSkill uses API 23+; guarded by FEATURE_CAMERA_FLASH check override fun build(ctx: SkillContext): Skill<*>? { val data = Sentences.Flashlight[ctx.sentencesLanguage] ?: return null if (!ctx.android.packageManager.hasSystemFeature(PackageManager.FEATURE_CAMERA_FLASH)) From 4220e9823a5f0a6e67bcc7197cfee964ace7ddb9 Mon Sep 17 00:00:00 2001 From: mark Date: Wed, 13 May 2026 17:36:22 +0100 Subject: [PATCH 8/8] Address review: pass yesNoData to TelephoneSkill/TimerSkill, revert settings screen - TelephoneSkill/TimerSkill: accept yesNoData as constructor param, pass through to ConfirmCallOutput/ConfirmCancel to eliminate runtime !! - SkillSettingsScreen: revert to build() != null for availability check with comment noting potential performance concern - Remove enabledSkillsInfo from ViewModel (no longer needed) --- .../stypox/dicio/settings/SkillSettingsScreen.kt | 15 ++++----------- .../dicio/settings/SkillSettingsViewModel.kt | 2 -- .../dicio/skills/telephone/ConfirmCallOutput.kt | 7 ++++--- .../dicio/skills/telephone/ContactChooserIndex.kt | 11 ++++++++--- .../dicio/skills/telephone/ContactChooserName.kt | 9 +++++++-- .../dicio/skills/telephone/TelephoneInfo.kt | 4 ++-- .../dicio/skills/telephone/TelephoneOutput.kt | 9 +++++++-- .../dicio/skills/telephone/TelephoneSkill.kt | 14 +++++++++----- .../org/stypox/dicio/skills/timer/TimerInfo.kt | 4 ++-- .../org/stypox/dicio/skills/timer/TimerOutput.kt | 5 +++-- .../org/stypox/dicio/skills/timer/TimerSkill.kt | 10 +++++++--- .../dicio/ui/util/PreviewParameterProviders.kt | 3 ++- 12 files changed, 55 insertions(+), 38 deletions(-) diff --git a/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsScreen.kt b/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsScreen.kt index a06b0f12e..6b8e5c36c 100644 --- a/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsScreen.kt +++ b/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsScreen.kt @@ -89,7 +89,6 @@ fun SkillSettingsScreen( ) { val skills = viewModel.skills val enabledSkills by viewModel.enabledSkills.collectAsState() - val enabledSkillsInfo by viewModel.enabledSkillsInfo.collectAsState() LazyColumn( contentPadding = PaddingValues(top = 4.dp, bottom = 4.dp), @@ -114,18 +113,12 @@ fun SkillSettingsScreen( } } items(skills) { skill -> - // enabledSkillsInfo contains skills that are both enabled AND available. - // A skill is "available" if: - // - it's in enabledSkillsInfo (enabled + buildable), OR - // - it's disabled by the user (we can't tell availability from the list, - // so we assume available — the worst case is a disabled skill shows as - // available, which is fine since it's toggled off anyway) - val isDisabledByUser = !enabledSkills.getOrDefault(skill.id, true) - val isAvailable = isDisabledByUser - || enabledSkillsInfo?.contains(skill) != false + // Note: calling build() here is slightly wasteful as it constructs a skill object + // just to check availability, but it ensures correct results regardless of whether + // the skill is enabled or disabled by the user. SkillSettingsItem( skill = skill, - isAvailable = isAvailable, + isAvailable = skill.build(viewModel.skillContext) != null, enabled = enabledSkills.getOrDefault(skill.id, true), setEnabled = { enabled -> viewModel.setSkillEnabled(skill.id, enabled) } ) diff --git a/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsViewModel.kt b/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsViewModel.kt index 1f5c2d853..8ed87ab4d 100644 --- a/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsViewModel.kt +++ b/app/src/main/kotlin/org/stypox/dicio/settings/SkillSettingsViewModel.kt @@ -25,8 +25,6 @@ class SkillSettingsViewModel @Inject constructor( val skills: List get() = skillHandler.allSkillInfoList - val enabledSkillsInfo = skillHandler.enabledSkillsInfo - // run blocking because the settings screen cannot start if settings have not been loaded yet val enabledSkills = dataStore.data .map { it.enabledSkillsMap } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/telephone/ConfirmCallOutput.kt b/app/src/main/kotlin/org/stypox/dicio/skills/telephone/ConfirmCallOutput.kt index d209571ec..e6ad428da 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/telephone/ConfirmCallOutput.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/telephone/ConfirmCallOutput.kt @@ -9,6 +9,7 @@ import androidx.compose.ui.unit.dp import org.dicio.skill.context.SkillContext import org.dicio.skill.skill.InteractionPlan import org.dicio.skill.skill.SkillOutput +import org.dicio.skill.standard.StandardRecognizerData import org.stypox.dicio.R import org.stypox.dicio.io.graphical.Body import org.stypox.dicio.io.graphical.Headline @@ -18,14 +19,14 @@ import org.stypox.dicio.util.getString class ConfirmCallOutput( private val name: String, - private val number: String + private val number: String, + private val yesNoData: StandardRecognizerData, ) : SkillOutput { override fun getSpeechOutput(ctx: SkillContext): String = ctx.getString(R.string.skill_telephone_confirm_call, name) override fun getInteractionPlan(ctx: SkillContext): InteractionPlan { - val yesNoSentences = Sentences.UtilYesNo[ctx.sentencesLanguage]!! - val confirmYesNoSkill = object : RecognizeYesNoSkill(TelephoneInfo, yesNoSentences) { + val confirmYesNoSkill = object : RecognizeYesNoSkill(TelephoneInfo, yesNoData) { override suspend fun generateOutput( ctx: SkillContext, inputData: Boolean diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/telephone/ContactChooserIndex.kt b/app/src/main/kotlin/org/stypox/dicio/skills/telephone/ContactChooserIndex.kt index 8d6367392..31d787cb2 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/telephone/ContactChooserIndex.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/telephone/ContactChooserIndex.kt @@ -8,8 +8,13 @@ import org.dicio.skill.skill.Skill import org.dicio.skill.skill.SkillOutput import org.dicio.skill.skill.Specificity -class ContactChooserIndex internal constructor(private val contacts: List>) : - Skill(TelephoneInfo, Specificity.HIGH) { +import org.dicio.skill.standard.StandardRecognizerData +import org.stypox.dicio.sentences.Sentences + +class ContactChooserIndex internal constructor( + private val contacts: List>, + private val yesNoData: StandardRecognizerData, +) : Skill(TelephoneInfo, Specificity.HIGH) { override fun score( ctx: SkillContext, @@ -29,7 +34,7 @@ class ContactChooserIndex internal constructor(private val contacts: List 0 && inputData <= contacts.size) { val contact = contacts[inputData - 1] - return ConfirmCallOutput(contact.first, contact.second) + return ConfirmCallOutput(contact.first, contact.second, yesNoData) } else { // impossible situation return ConfirmedCallOutput(null) diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/telephone/ContactChooserName.kt b/app/src/main/kotlin/org/stypox/dicio/skills/telephone/ContactChooserName.kt index dbad71756..3a66a962b 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/telephone/ContactChooserName.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/telephone/ContactChooserName.kt @@ -7,9 +7,14 @@ import org.dicio.skill.skill.Score import org.dicio.skill.skill.Skill import org.dicio.skill.skill.SkillOutput import org.dicio.skill.skill.Specificity +import org.dicio.skill.standard.StandardRecognizerData +import org.stypox.dicio.sentences.Sentences import org.stypox.dicio.util.StringUtils -class ContactChooserName internal constructor(private val contacts: List>) : +class ContactChooserName internal constructor( + private val contacts: List>, + private val yesNoData: StandardRecognizerData, +) : // use a low specificity to prefer the index-based contact chooser Skill?>(TelephoneInfo, Specificity.LOW) { @@ -38,7 +43,7 @@ class ContactChooserName internal constructor(private val contacts: List?): SkillOutput { return inputData?.let { - ConfirmCallOutput(it.first, it.second) + ConfirmCallOutput(it.first, it.second, yesNoData) } // impossible situation ?: ConfirmedCallOutput(null) diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneInfo.kt index 2312b6d40..400f2ce07 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneInfo.kt @@ -33,7 +33,7 @@ object TelephoneInfo : SkillInfo("telephone") { override fun build(ctx: SkillContext): Skill<*>? { val data = Sentences.Telephone[ctx.sentencesLanguage] ?: return null - Sentences.UtilYesNo[ctx.sentencesLanguage] ?: return null - return TelephoneSkill(TelephoneInfo, data) + val yesNoData = Sentences.UtilYesNo[ctx.sentencesLanguage] ?: return null + return TelephoneSkill(TelephoneInfo, data, yesNoData) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneOutput.kt b/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneOutput.kt index 5a5b4cb4c..283a66f5b 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneOutput.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneOutput.kt @@ -13,12 +13,15 @@ import org.dicio.skill.skill.Skill import org.dicio.skill.context.SkillContext import org.dicio.skill.skill.InteractionPlan import org.dicio.skill.skill.SkillOutput +import org.dicio.skill.standard.StandardRecognizerData import org.stypox.dicio.R import org.stypox.dicio.io.graphical.Headline +import org.stypox.dicio.sentences.Sentences import org.stypox.dicio.util.getString class TelephoneOutput( private val contacts: List>>, + private val yesNoData: StandardRecognizerData, ) : SkillOutput { override fun getSpeechOutput(ctx: SkillContext): String = if (contacts.isEmpty()) { ctx.getString(R.string.skill_telephone_unknown_contact) @@ -31,7 +34,8 @@ class TelephoneOutput( ContactChooserName( // when saying the name, there is no way to distinguish between // different numbers, so just use the first one - contacts.map { Pair(it.first, it.second[0]) } + contacts.map { Pair(it.first, it.second[0]) }, + yesNoData, ) ) @@ -42,7 +46,8 @@ class TelephoneOutput( contact.second.map { number -> Pair(contact.first, number) } - } + }, + yesNoData, ) ) } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneSkill.kt b/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneSkill.kt index 65ebf227b..40106383f 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneSkill.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/telephone/TelephoneSkill.kt @@ -8,10 +8,14 @@ import org.dicio.skill.skill.SkillInfo import org.dicio.skill.skill.SkillOutput import org.dicio.skill.standard.StandardRecognizerData import org.dicio.skill.standard.StandardRecognizerSkill +import org.stypox.dicio.sentences.Sentences import org.stypox.dicio.sentences.Sentences.Telephone -class TelephoneSkill(correspondingSkillInfo: SkillInfo, data: StandardRecognizerData) : - StandardRecognizerSkill(correspondingSkillInfo, data) { +class TelephoneSkill( + correspondingSkillInfo: SkillInfo, + data: StandardRecognizerData, + val yesNoData: StandardRecognizerData, +) : StandardRecognizerSkill(correspondingSkillInfo, data) { override suspend fun generateOutput(ctx: SkillContext, inputData: Telephone): SkillOutput { val contentResolver = ctx.android.contentResolver @@ -36,7 +40,7 @@ class TelephoneSkill(correspondingSkillInfo: SkillInfo, data: StandardRecognizer || contacts[i + 1].distance - 2 > contact.distance) ) { // very close match with just one number and without distance ties: call it directly - return ConfirmCallOutput(contact.name, numbers[0]) + return ConfirmCallOutput(contact.name, numbers[0], yesNoData) } validContacts.add(Pair(contact.name, numbers)) ++i @@ -50,11 +54,11 @@ class TelephoneSkill(correspondingSkillInfo: SkillInfo, data: StandardRecognizer ) { // not a good enough match, but since we have only this, call it directly val contact = validContacts[0] - return ConfirmCallOutput(contact.first, contact.second[0]) + return ConfirmCallOutput(contact.first, contact.second[0], yesNoData) } // this point will not be reached if a very close match was found - return TelephoneOutput(validContacts) + return TelephoneOutput(validContacts, yesNoData) } companion object { diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerInfo.kt b/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerInfo.kt index 9ee05cee2..3f3cea765 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerInfo.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerInfo.kt @@ -26,8 +26,8 @@ object TimerInfo : SkillInfo("timer") { override fun build(ctx: SkillContext): Skill<*>? { val data = Sentences.Timer[ctx.sentencesLanguage] ?: return null - Sentences.UtilYesNo[ctx.sentencesLanguage] ?: return null + val yesNoData = Sentences.UtilYesNo[ctx.sentencesLanguage] ?: return null if (ctx.parserFormatter == null) return null - return TimerSkill(TimerInfo, data) + return TimerSkill(TimerInfo, data, yesNoData) } } diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerOutput.kt b/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerOutput.kt index 47fc4a661..cdf8959de 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerOutput.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerOutput.kt @@ -13,6 +13,7 @@ import org.dicio.skill.skill.Score import org.dicio.skill.skill.Skill import org.dicio.skill.skill.SkillOutput import org.dicio.skill.skill.Specificity +import org.dicio.skill.standard.StandardRecognizerData import org.stypox.dicio.R import org.stypox.dicio.io.graphical.Headline import org.stypox.dicio.io.graphical.HeadlineSpeechSkillOutput @@ -95,14 +96,14 @@ sealed interface TimerOutput : SkillOutput { } class ConfirmCancel( + private val yesNoData: StandardRecognizerData, private val onConfirm: () -> SkillOutput, ) : TimerOutput, HeadlineSpeechSkillOutput { override fun getSpeechOutput(ctx: SkillContext): String = ctx.getString(R.string.skill_timer_confirm_cancel) override fun getInteractionPlan(ctx: SkillContext): InteractionPlan { - val yesNoSentences = Sentences.UtilYesNo[ctx.sentencesLanguage]!! - val confirmYesNoSkill = object : RecognizeYesNoSkill(TimerInfo, yesNoSentences) { + val confirmYesNoSkill = object : RecognizeYesNoSkill(TimerInfo, yesNoData) { override suspend fun generateOutput( ctx: SkillContext, inputData: Boolean diff --git a/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerSkill.kt b/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerSkill.kt index b47a64162..c84f9779b 100644 --- a/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerSkill.kt +++ b/app/src/main/kotlin/org/stypox/dicio/skills/timer/TimerSkill.kt @@ -13,14 +13,18 @@ import org.dicio.skill.skill.SkillOutput import org.dicio.skill.standard.StandardRecognizerData import org.dicio.skill.standard.StandardRecognizerSkill import org.stypox.dicio.R +import org.stypox.dicio.sentences.Sentences import org.stypox.dicio.sentences.Sentences.Timer import org.stypox.dicio.util.StringUtils import org.stypox.dicio.util.getString import java.time.Duration // TODO cleanup this skill and use a service to manage timers -class TimerSkill(correspondingSkillInfo: SkillInfo, data: StandardRecognizerData) : - StandardRecognizerSkill(correspondingSkillInfo, data) { +class TimerSkill( + correspondingSkillInfo: SkillInfo, + data: StandardRecognizerData, + private val yesNoData: StandardRecognizerData, +) : StandardRecognizerSkill(correspondingSkillInfo, data) { override suspend fun generateOutput(ctx: SkillContext, inputData: Timer): SkillOutput { return when (inputData) { @@ -36,7 +40,7 @@ class TimerSkill(correspondingSkillInfo: SkillInfo, data: StandardRecognizerData } is Timer.Cancel -> { if (inputData.name == null && SET_TIMERS.size > 1) { - TimerOutput.ConfirmCancel { cancelTimer(ctx, null) } + TimerOutput.ConfirmCancel(yesNoData) { cancelTimer(ctx, null) } } else { cancelTimer(ctx, inputData.name) } diff --git a/app/src/main/kotlin/org/stypox/dicio/ui/util/PreviewParameterProviders.kt b/app/src/main/kotlin/org/stypox/dicio/ui/util/PreviewParameterProviders.kt index 9c8e0637b..969ff5adc 100644 --- a/app/src/main/kotlin/org/stypox/dicio/ui/util/PreviewParameterProviders.kt +++ b/app/src/main/kotlin/org/stypox/dicio/ui/util/PreviewParameterProviders.kt @@ -21,6 +21,7 @@ import org.stypox.dicio.skills.navigation.NavigationOutput import org.stypox.dicio.skills.telephone.ConfirmCallOutput import org.stypox.dicio.skills.telephone.ConfirmedCallOutput import org.stypox.dicio.skills.telephone.TelephoneInfo +import org.stypox.dicio.sentences.Sentences import org.stypox.dicio.skills.timer.TimerInfo import org.stypox.dicio.skills.timer.TimerOutput import org.stypox.dicio.skills.weather.WeatherInfo @@ -105,7 +106,7 @@ class InteractionLogPreviews : CollectionPreviewParameterProvider