demus/openhab-core
1
0
Fork 0
mirror of https://github.com/danieldemus/openhab-core.git synced 2026-07-29 12:34:22 +02:00
Code Issues 1 Packages Projects Releases Wiki Activity

[voice] Address feedback from PR #5620 (#5632)

Browse Source 
* [voice] ConversationManager: Don't create "anonymous" conversation if createIfMissing=false in getConversation()
* [voice] Fix SAT & null warnings
* Adjust ConversationMapperTest to DTO changes

Signed-off-by: Florian Hotze <dev@florianhotze.com>
This commit is contained in:
Florian Hotze
2026-06-06 14:36:09 +02:00
committed by GitHub
parent 05b7c180d5
commit 9e72e1a207
21 changed files with 102 additions and 111 deletions
Download Patch File Download Diff File Expand all files Collapse all files
bundles/org.openhab.core.io.rest.voice/src/main/java/org/openhab/core/io/rest/voice/internal/ConversationDTO.java
+2 -1
View File
@@ -16,6 +16,7 @@ import java.util.List;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.openhab.core.voice.text.conversation.Conversation;
import org.openhab.core.voice.text.conversation.ConversationRole;
import io.swagger.v3.oas.annotations.media.Schema;
@@ -28,6 +29,6 @@ import io.swagger.v3.oas.annotations.media.Schema;
@NonNullByDefault
public record ConversationDTO(String id, List<MessageDTO> messages) {
@Schema(name = "Message")
public record MessageDTO(int id, String role, String content) {
public record MessageDTO(int id, ConversationRole role, String content) {
}
}
bundles/org.openhab.core.io.rest.voice/src/main/java/org/openhab/core/io/rest/voice/internal/ConversationMapper.java
+1 -1
View File
@@ -32,6 +32,6 @@ public class ConversationMapper {
*/
public static ConversationDTO map(Conversation conversation) {
return new ConversationDTO(conversation.getId(), conversation.getMessages().stream()
.map(m -> new ConversationDTO.MessageDTO(m.id(), m.role().name(), m.content())).toList());
.map(m -> new ConversationDTO.MessageDTO(m.id(), m.role(), m.content())).toList());
}
}
bundles/org.openhab.core.io.rest.voice/src/test/java/org/openhab/core/io/rest/voice/internal/ConversationMapperTest.java
+2 -2
View File
@@ -39,9 +39,9 @@ public class ConversationMapperTest {
assertNotNull(dto);
assertEquals(id, dto.id());
assertEquals(2, dto.messages().size());
assertEquals(ConversationRole.USER.name(), dto.messages().get(0).role());
assertEquals(ConversationRole.USER, dto.messages().get(0).role());
assertEquals("Hello", dto.messages().get(0).content());
assertEquals(ConversationRole.OPENHAB.name(), dto.messages().get(1).role());
assertEquals(ConversationRole.OPENHAB, dto.messages().get(1).role());
assertEquals("Hi there!", dto.messages().get(1).content());
}
}
bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/DialogContext.java
+8 -6
View File
@@ -22,6 +22,7 @@ import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.audio.AudioSink;
import org.openhab.core.audio.AudioSource;
import org.openhab.core.voice.text.HumanLanguageInterpreter;
import org.openhab.core.voice.text.conversation.Conversation;
import org.openhab.core.voice.text.interpreter.llm.LLMTool;
/**
@@ -33,7 +34,7 @@ import org.openhab.core.voice.text.interpreter.llm.LLMTool;
public record DialogContext(@Nullable DTService dt, @Nullable String keyword, STTService stt, TTSService tts,
@Nullable Voice voice, List<HumanLanguageInterpreter> hlis, AudioSource source, AudioSink sink, Locale locale,
String dialogGroup, @Nullable String locationItem, @Nullable String listeningItem,
@Nullable String listeningMelody, @Nullable String conversationId, List<LLMTool> llmTools) {
@Nullable String listeningMelody, Conversation conversation, List<LLMTool> llmTools) {
/**
* Builder for {@link DialogContext}
@@ -49,8 +50,9 @@ public record DialogContext(@Nullable DTService dt, @Nullable String keyword, ST
private @Nullable Voice voice;
private List<HumanLanguageInterpreter> hlis = List.of();
private List<LLMTool> llmTools = List.of();
// state
private Conversation conversation = new Conversation("");
// options
private @Nullable String conversationId;
private String dialogGroup = "default";
private @Nullable String locationItem;
private @Nullable String listeningItem;
@@ -133,9 +135,9 @@ public record DialogContext(@Nullable DTService dt, @Nullable String keyword, ST
return this;
}
public Builder withConversationId(@Nullable String conversationId) {
if (conversationId != null) {
this.conversationId = conversationId;
public Builder withConversation(@Nullable Conversation conversation) {
if (conversation != null) {
this.conversation = conversation;
}
return this;
}
@@ -216,7 +218,7 @@ public record DialogContext(@Nullable DTService dt, @Nullable String keyword, ST
throw new IllegalStateException("Cannot build dialog context: " + String.join(", ", errors) + ".");
} else {
return new DialogContext(dtService, keyword, sttService, ttsService, voice, hliServices, audioSource,
audioSink, locale, dialogGroup, locationItem, listeningItem, listeningMelody, conversationId,
audioSink, locale, dialogGroup, locationItem, listeningItem, listeningMelody, conversation,
llmTools);
}
}
bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/SpeechRecognitionEvent.java
+2 -2
View File
@@ -42,8 +42,8 @@ public class SpeechRecognitionEvent implements STTEvent {
* @param confidence The confidence of the transcript
*/
public SpeechRecognitionEvent(String transcript, float confidence) {
if ((null == transcript) || (transcript.isEmpty())) {
throw new IllegalArgumentException("The passed transcript is null or empty");
if (transcript.isEmpty()) {
throw new IllegalArgumentException("The passed transcript is empty");
}
if ((confidence < 0.0) || (1.0 < confidence)) {
throw new IllegalArgumentException("The passed confidence is less than 0.0 or greater than 1.0");
bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/AudioDialogProviderImpl.java
+3 -5
View File
@@ -28,8 +28,6 @@ import org.openhab.core.voice.VoiceManager;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Reference;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* Allows the audio bundle to register a dialog that can be triggered programmatically.
@@ -39,7 +37,6 @@ import org.slf4j.LoggerFactory;
@Component(service = AudioDialogProvider.class)
@NonNullByDefault
public class AudioDialogProviderImpl implements AudioDialogProvider {
private final Logger logger = LoggerFactory.getLogger(AudioDialogProviderImpl.class);
private final VoiceManager voiceManager;
@Activate
@@ -106,8 +103,9 @@ public class AudioDialogProviderImpl implements AudioDialogProvider {
@Override
public void abort() {
if (this.abortCallback != null) {
this.abortCallback.run();
var abortCallback = this.abortCallback;
if (abortCallback != null) {
abortCallback.run();
this.abortCallback = null;
}
}
bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/DialogProcessor.java
+2 -9
View File
@@ -62,7 +62,6 @@ import org.openhab.core.voice.text.InterpretationException;
import org.openhab.core.voice.text.InterpreterContext;
import org.openhab.core.voice.text.conversation.Conversation;
import org.openhab.core.voice.text.conversation.ConversationException;
import org.openhab.core.voice.text.conversation.ConversationManager;
import org.openhab.core.voice.text.conversation.ConversationRole;
import org.openhab.core.voice.text.interpreter.llm.LLMTool;
import org.osgi.framework.Bundle;
@@ -89,7 +88,6 @@ public class DialogProcessor implements KSListener, STTListener {
private final Logger logger = LoggerFactory.getLogger(DialogProcessor.class);
private final WeakHashMap<String, DialogContext> activeDialogGroups;
public final DialogContext dialogContext;
private final ConversationManager conversationManager;
private @Nullable List<ToneSynthesizer.Tone> listeningMelody;
private final EventPublisher eventPublisher;
private final TranslationProvider i18nProvider;
@@ -118,13 +116,11 @@ public class DialogProcessor implements KSListener, STTListener {
private @Nullable ToneSynthesizer toneSynthesizer;
public DialogProcessor(DialogContext context, DialogEventListener eventListener, EventPublisher eventPublisher,
WeakHashMap<String, DialogContext> activeDialogGroups, TranslationProvider i18nProvider,
ConversationManager conversationManager, Bundle bundle) {
WeakHashMap<String, DialogContext> activeDialogGroups, TranslationProvider i18nProvider, Bundle bundle) {
this.dialogContext = context;
this.eventListener = eventListener;
this.eventPublisher = eventPublisher;
this.i18nProvider = i18nProvider;
this.conversationManager = conversationManager;
this.activeDialogGroups = activeDialogGroups;
this.bundle = bundle;
var dt = context.dt();
@@ -367,10 +363,7 @@ public class DialogProcessor implements KSListener, STTListener {
logger.debug("Text recognized: {}", question);
toggleProcessing(false);
eventListener.onBeforeDialogInterpretation(dialogContext);
@Nullable
String conversationId = dialogContext.conversationId();
Conversation conversation = conversationManager
.getConversation(Objects.requireNonNullElse(conversationId, ""));
Conversation conversation = dialogContext.conversation();
String error = null;
String answer = "";
try {
bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceConfiguration.java
+11 -29
View File
@@ -18,9 +18,8 @@ import java.util.Map.Entry;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.config.core.ConfigParser;
import org.openhab.core.voice.text.conversation.Conversation;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* The {@link VoiceConfiguration} class holds the configuration for the {@link VoiceManagerImpl}.
@@ -45,8 +44,6 @@ public class VoiceConfiguration {
public static final String CONFIG_PREFIX_DEFAULT_VOICE = "defaultVoice.";
public static final String CONFIG_CONVERSATION_HISTORY_LIMIT = "conversationHistoryLimit";
private final Logger logger = LoggerFactory.getLogger(VoiceConfiguration.class);
private String keyword = DEFAULT_KEYWORD;
private @Nullable String listeningItem;
private @Nullable String listeningMelody;
@@ -59,31 +56,16 @@ public class VoiceConfiguration {
private final Map<String, String> defaultVoices = new HashMap<>();
public void update(Map<String, Object> config) {
this.keyword = config.containsKey(CONFIG_KEYWORD) ? config.get(CONFIG_KEYWORD).toString() : DEFAULT_KEYWORD;
this.listeningItem = config.containsKey(CONFIG_LISTENING_ITEM) ? config.get(CONFIG_LISTENING_ITEM).toString()
: null;
this.listeningMelody = config.containsKey(CONFIG_LISTENING_MELODY)
? config.get(CONFIG_LISTENING_MELODY).toString()
: null;
this.defaultTTS = config.containsKey(CONFIG_DEFAULT_TTS) ? config.get(CONFIG_DEFAULT_TTS).toString() : null;
this.defaultSTT = config.containsKey(CONFIG_DEFAULT_STT) ? config.get(CONFIG_DEFAULT_STT).toString() : null;
this.defaultKS = config.containsKey(CONFIG_DEFAULT_KS) ? config.get(CONFIG_DEFAULT_KS).toString() : null;
this.defaultHLI = config.containsKey(CONFIG_DEFAULT_HLI) ? config.get(CONFIG_DEFAULT_HLI).toString() : null;
this.defaultVoice = config.containsKey(CONFIG_DEFAULT_VOICE) ? config.get(CONFIG_DEFAULT_VOICE).toString()
: null;
if (config.containsKey(CONFIG_CONVERSATION_HISTORY_LIMIT)) {
try {
this.conversationHistoryLimit = Integer
.parseInt(config.get(CONFIG_CONVERSATION_HISTORY_LIMIT).toString());
} catch (NumberFormatException e) {
logger.warn("Failed to parse {}, setting to default ({}):", CONFIG_CONVERSATION_HISTORY_LIMIT,
Conversation.DEFAULT_MAX_MESSAGES, e);
this.conversationHistoryLimit = Conversation.DEFAULT_MAX_MESSAGES;
}
} else {
this.conversationHistoryLimit = Conversation.DEFAULT_MAX_MESSAGES;
}
this.keyword = ConfigParser.valueAsOrElse(config.get(CONFIG_KEYWORD), String.class, DEFAULT_KEYWORD);
this.listeningItem = ConfigParser.valueAs(config.get(CONFIG_LISTENING_ITEM), String.class);
this.listeningMelody = ConfigParser.valueAs(config.get(CONFIG_LISTENING_MELODY), String.class);
this.defaultTTS = ConfigParser.valueAs(config.get(CONFIG_DEFAULT_TTS), String.class);
this.defaultSTT = ConfigParser.valueAs(config.get(CONFIG_DEFAULT_STT), String.class);
this.defaultKS = ConfigParser.valueAs(config.get(CONFIG_DEFAULT_KS), String.class);
this.defaultHLI = ConfigParser.valueAs(config.get(CONFIG_DEFAULT_HLI), String.class);
this.defaultVoice = ConfigParser.valueAs(config.get(CONFIG_DEFAULT_VOICE), String.class);
this.conversationHistoryLimit = ConfigParser.valueAsOrElse(config.get(CONFIG_CONVERSATION_HISTORY_LIMIT),
Integer.class, Conversation.DEFAULT_MAX_MESSAGES);
defaultVoices.clear();
for (Entry<String, Object> entry : config.entrySet()) {
bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceConsoleCommandExtension.java
+3 -2
View File
@@ -313,7 +313,6 @@ public class VoiceConsoleCommandExtension extends AbstractConsoleCommandExtensio
}
private void interpret(String[] args, Console console) {
HashMap<String, String> parameters;
try {
parameters = parseNamedParameters(args, true);
@@ -654,6 +653,8 @@ public class VoiceConsoleCommandExtension extends AbstractConsoleCommandExtensio
}
dialogContextBuilder.withSink(sink);
}
Conversation conversation = conversationManager
.getConversation(Objects.requireNonNullElse(parameters.remove("conversation"), ""));
dialogContextBuilder //
.withSTT(voiceManager.getSTT(parameters.remove("stt"))) //
.withTTS(voiceManager.getTTS(parameters.remove("tts"))) //
@@ -663,7 +664,7 @@ public class VoiceConsoleCommandExtension extends AbstractConsoleCommandExtensio
.withListeningItem(parameters.remove("listening-item")) //
.withLocationItem(parameters.remove("location-item")) //
.withDialogGroup(parameters.remove("dialog-group")) //
.withConversationId(parameters.remove("conversation")) //
.withConversation(conversation) //
.withLLMTools(llmToolRegistry.getByIds(parameters.remove("llm-tools"))) //
.withKeyword(parameters.remove("keyword"));
if (!parameters.isEmpty()) {
bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java
+9 -4
View File
@@ -349,7 +349,9 @@ public class VoiceManagerImpl implements VoiceManager, ConfigOptionProvider, Dia
logger.warn("InterruptedException waiting for transcription: {}", e.getMessage());
sttServiceHandle.abort();
} catch (ExecutionException e) {
logger.warn("ExecutionException running transcription: {}", e.getCause().getMessage());
var cause = e.getCause();
logger.warn("ExecutionException running transcription: {}",
cause != null ? cause.getMessage() : e.getMessage());
} catch (TimeoutException e) {
logger.warn("TimeoutException waiting for transcription");
sttServiceHandle.abort();
@@ -593,6 +595,7 @@ public class VoiceManagerImpl implements VoiceManager, ConfigOptionProvider, Dia
return new DialogContext.Builder(configuration.getKeyword(), localeProvider.getLocale()) //
.withSink(audioManager.getSink()) //
.withSource(audioManager.getSource()) //
.withConversation(conversationManager.getConversation("")) //
.withKS(this.getKS()) //
.withSTT(this.getSTT()) //
.withTTS(this.getTTS()) //
@@ -639,7 +642,7 @@ public class VoiceManagerImpl implements VoiceManager, ConfigOptionProvider, Dia
logger.debug("Starting a new dialog for source {} ({})", context.source().getLabel(null),
context.source().getId());
processor = new DialogProcessor(context, this, this.eventPublisher, this.activeDialogGroups,
this.i18nProvider, this.conversationManager, b);
this.i18nProvider, b);
dialogProcessors.put(context.source().getId(), processor);
return processor.start();
} else {
@@ -696,7 +699,7 @@ public class VoiceManagerImpl implements VoiceManager, ConfigOptionProvider, Dia
activeProcessor = singleDialogProcessors.get(audioSource.getId());
}
var processor = new DialogProcessor(context, this, this.eventPublisher, this.activeDialogGroups,
this.i18nProvider, this.conversationManager, b);
this.i18nProvider, b);
if (activeProcessor == null) {
logger.debug("Executing a simple dialog for source {} ({})", audioSource.getLabel(null),
audioSource.getId());
@@ -1086,6 +1089,8 @@ public class VoiceManagerImpl implements VoiceManager, ConfigOptionProvider, Dia
dialogRegistrationStorage.getValues().forEach(dr -> {
if (dr != null && !dialogProcessors.containsKey(dr.sourceId)) {
try {
Conversation conversation = conversationManager
.getConversation(Objects.requireNonNullElse(dr.conversationId, ""));
startDialog(getDialogContextBuilder() //
.withSink(audioManager.getSink(dr.sinkId)) //
.withSource(audioManager.getSource(dr.sourceId)) //
@@ -1098,7 +1103,7 @@ public class VoiceManagerImpl implements VoiceManager, ConfigOptionProvider, Dia
.withLLMTools(llmToolRegistry.getByIds(dr.llmToolIds)) //
.withLocale(dr.locale) //
.withDialogGroup(dr.dialogGroup) //
.withConversationId(dr.conversationId) //
.withConversation(conversation) //
.withLocationItem(dr.locationItem) //
.withListeningItem(dr.listeningItem) //
.withMelody(dr.listeningMelody) //
bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/cache/TTSLRUCacheImpl.java
Vendored
+1 -2
View File
@@ -27,7 +27,6 @@ import org.openhab.core.config.core.ConfigParser;
import org.openhab.core.storage.StorageService;
import org.openhab.core.voice.TTSCache;
import org.openhab.core.voice.TTSException;
import org.openhab.core.voice.TTSService;
import org.openhab.core.voice.Voice;
import org.openhab.core.voice.internal.VoiceManagerImpl;
import org.osgi.service.component.annotations.Activate;
@@ -38,7 +37,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* Cache system to avoid requesting {@link TTSService} for the same utterances.
* Cache system to avoid requesting {@link org.openhab.core.voice.TTSService} for the same utterances.
* This is a LRU cache (least recently used entry is evicted if the size
* is exceeded)
* Size is based on the size on disk (in bytes)
bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/text/conversation/ConversationManagerImpl.java
+8 -7
View File
@@ -41,7 +41,7 @@ import org.slf4j.LoggerFactory;
@Component(service = ConversationManager.class)
@NonNullByDefault
public class ConversationManagerImpl implements ConversationManager, ConversationListener {
private final Storage<ConversationDTO> conversationStorage;
private final Storage<PersistedConversationDTO> conversationStorage;
private final Map<String, Conversation> activeConversations = new ConcurrentHashMap<>();
private final EventPublisher eventPublisher;
private final Logger logger = LoggerFactory.getLogger(ConversationManagerImpl.class);
@@ -58,7 +58,8 @@ public class ConversationManagerImpl implements ConversationManager, Conversatio
@Override
public @Nullable Conversation getConversation(String id, boolean createIfMissing) {
Conversation conversation;
if (id.isBlank()) {
if (id.isBlank() && createIfMissing) {
logger.debug("Creating new unpersisted conversation");
conversation = new Conversation("");
conversation.setMaxMessages(historyLimit);
return conversation;
@@ -78,11 +79,11 @@ public class ConversationManagerImpl implements ConversationManager, Conversatio
return conversation;
}
// load conversation from storage or create a new one
ConversationDTO conversationDTO = conversationStorage.get(id);
if (conversationDTO != null) {
PersistedConversationDTO persistedConversationDTO = conversationStorage.get(id);
if (persistedConversationDTO != null) {
logger.debug("Conversation '{}' found", id);
conversation = new Conversation(id,
conversationDTO.messages().stream().map(MessageDTO::toMessage).toList());
persistedConversationDTO.messages().stream().map(PersistedMessageDTO::toMessage).toList());
} else if (createIfMissing) {
logger.debug("Creating new conversation '{}'", id);
conversation = new Conversation(id);
@@ -115,8 +116,8 @@ public class ConversationManagerImpl implements ConversationManager, Conversatio
removeConversation(id);
} else {
logger.debug("Storing conversation '{}' with {} messages...", id, conversation.getMessages().size());
conversationStorage.put(id, new ConversationDTO(
new ArrayList<>(conversation.getMessages().stream().map(MessageDTO::fromMessage).toList())));
conversationStorage.put(id, new PersistedConversationDTO(id, new ArrayList<>(
conversation.getMessages().stream().map(PersistedMessageDTO::fromMessage).toList())));
activeConversations.put(id, conversation);
}
}
bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/text/conversation/ConversationDTO.java → bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/text/conversation/PersistedConversationDTO.java
+4 -4
View File
@@ -15,15 +15,15 @@ package org.openhab.core.voice.internal.text.conversation;
import java.util.List;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.openhab.core.voice.text.conversation.Conversation;
/**
* The {@link ConversationDTO} class contains a list of messages in between the users and a LanguageInterpreter.
* It is used to store {@link Conversation}s using a
* The {@link PersistedConversationDTO} class contains a list of messages in between the users and a
* LanguageInterpreter.
* It is used to store {@link org.openhab.core.voice.text.conversation.Conversation}s using a
* {@link org.openhab.core.storage.StorageService}.
*
* @author Miguel Álvarez Díez - Initial contribution
*/
@NonNullByDefault
public record ConversationDTO(List<MessageDTO> messages) {
public record PersistedConversationDTO(String id, List<PersistedMessageDTO> messages) {
}
bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/text/conversation/MessageDTO.java → bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/text/conversation/PersistedMessageDTO.java
+4 -4
View File
@@ -17,19 +17,19 @@ import org.openhab.core.voice.text.conversation.Conversation;
import org.openhab.core.voice.text.conversation.ConversationRole;
/**
* The {@link MessageDTO} class represents a message in between the users and a LanguageInterpreter.
* The {@link PersistedMessageDTO} class represents a message in between the users and a LanguageInterpreter.
* It is used to store {@link Conversation}s using a
* {@link org.openhab.core.storage.StorageService}.
*
* @author Miguel Álvarez Díez - Initial contribution
*/
@NonNullByDefault
public record MessageDTO(int id, ConversationRole role, String content) {
public record PersistedMessageDTO(int id, ConversationRole role, String content) {
public Conversation.Message toMessage() {
return new Conversation.Message(id, role, content);
}
public static MessageDTO fromMessage(Conversation.Message messageRecord) {
return new MessageDTO(messageRecord.id(), messageRecord.role(), messageRecord.content());
public static PersistedMessageDTO fromMessage(Conversation.Message messageRecord) {
return new PersistedMessageDTO(messageRecord.id(), messageRecord.role(), messageRecord.content());
}
}
bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/text/interpreter/StandardInterpreter.java
-3
View File
@@ -49,8 +49,6 @@ import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Deactivate;
import org.osgi.service.component.annotations.Reference;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* A human language command interpretation service.
@@ -65,7 +63,6 @@ import org.slf4j.LoggerFactory;
@Component(service = HumanLanguageInterpreter.class)
public class StandardInterpreter extends AbstractRuleBasedInterpreter {
public static final String VOICE_SYSTEM_NAMESPACE = "voiceSystem";
private Logger logger = LoggerFactory.getLogger(StandardInterpreter.class);
private final ItemRegistry itemRegistry;
private final MetadataRegistry metadataRegistry;
bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/text/conversation/ConversationManager.java
+1 -2
View File
@@ -27,14 +27,13 @@ import org.eclipse.jdt.annotation.Nullable;
* <li>Automatically persist a persistable {@link Conversation} whenever a message is added or messages are
* removed.</li>
* <li>Emit {@link org.openhab.core.voice.text.conversation.events.ConversationEvent} implementations accordingly.</li>
* <li>If a conversation has a blank ID, no events will be emitted.</li>
* <li>If a conversation has a blank ID, it must not be persisted and events must not be emitted.</li>
* </ul>
*
* @author Florian Hotze - Initial contribution
*/
@NonNullByDefault
public interface ConversationManager {
/**
* Gets a conversation by its identifier.
*
bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/text/conversation/events/ConversationEvent.java
+1 -1
View File
@@ -39,7 +39,7 @@ public abstract class ConversationEvent extends AbstractEvent {
}
@Override
abstract public String getType();
public abstract String getType();
public String getConversationId() {
return conversationId;
bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/text/interpreter/rulebased/AbstractRuleBasedInterpreter.java
+5 -2
View File
@@ -229,8 +229,11 @@ public abstract class AbstractRuleBasedInterpreter implements HumanLanguageInter
}
}
}
if (lastResult != null && lastResult.getException() != null) {
throw lastResult.getException();
if (lastResult != null) {
var lastException = lastResult.getException();
if (lastException != null) {
throw lastException;
}
}
throw new InterpretationException(language.getString(SORRY));
}
bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/text/interpreter/rulebased/TokenList.java
+13 -9
View File
@@ -14,17 +14,21 @@ package org.openhab.core.voice.text.interpreter.rulebased;
import java.util.List;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
/**
* A helper to parse a sequence of tokens. This class is immutable.
*
* @author Tilman Kamp - Initial contribution
*/
@NonNullByDefault
public class TokenList {
private List<String> list;
private final List<String> list;
private int head;
private int tail;
private final int head;
private final int tail;
/**
* Constructs a new instance.
@@ -48,7 +52,7 @@ public class TokenList {
*
* @return the first token of the list
*/
public String head() {
public @Nullable String head() {
return (list.isEmpty() || head < 0 || head >= list.size()) ? null : list.get(head);
}
@@ -57,7 +61,7 @@ public class TokenList {
*
* @return the last token of the list
*/
public String tail() {
public @Nullable String tail() {
return (list.isEmpty() || tail < 0 || tail >= list.size()) ? null : list.get(tail);
}
@@ -113,7 +117,7 @@ public class TokenList {
* @return First token, if it is equal to one of the alternatives or if no alternatives were provided.
* Null otherwise. Always null, if there is no first token (if the list is empty).
*/
public String peekHead(String... alternatives) {
public @Nullable String peekHead(String... alternatives) {
return peek(head, alternatives);
}
@@ -125,7 +129,7 @@ public class TokenList {
* @return Last token, if it is equal to one of the alternatives or if no alternatives were provided.
* Null otherwise. Always null, if there is no last token (if the list is empty).
*/
public String peekTail(String... alternatives) {
public @Nullable String peekTail(String... alternatives) {
return peek(tail, alternatives);
}
@@ -147,7 +151,7 @@ public class TokenList {
return new TokenList(list, head, tail - 1);
}
private String peek(int index, String... alternatives) {
private @Nullable String peek(int index, String... alternatives) {
return splice(index, alternatives);
}
@@ -155,7 +159,7 @@ public class TokenList {
return splice(index, alternatives) != null;
}
private String splice(int index, String... alternatives) {
private @Nullable String splice(int index, String... alternatives) {
if (index < head || index > tail || head > tail) {
return null;
}
bundles/org.openhab.core.voice/src/test/java/org/openhab/core/voice/internal/text/conversation/ConversationManagerImplTest.java
+11 -11
View File
@@ -47,7 +47,7 @@ public class ConversationManagerImplTest {
private final StorageService storageService = mock(StorageService.class);
private final EventPublisher eventPublisher = mock(EventPublisher.class);
@SuppressWarnings("unchecked")
private final Storage<ConversationDTO> storage = mock(Storage.class);
private final Storage<PersistedConversationDTO> storage = mock(Storage.class);
private @NonNullByDefault({}) ConversationManagerImpl conversationManager;
@@ -106,10 +106,10 @@ public class ConversationManagerImplTest {
@Test
public void getConversationLoadsConversationFromStorage() {
String id = "stored-conv";
MessageDTO messageDTO = new MessageDTO(1, ConversationRole.USER, "Hello");
ArrayList<MessageDTO> messages = new ArrayList<>(List.of(messageDTO));
ConversationDTO conversationDTO = new ConversationDTO(messages);
when(storage.get(id)).thenReturn(conversationDTO);
PersistedMessageDTO persistedMessageDTO = new PersistedMessageDTO(1, ConversationRole.USER, "Hello");
ArrayList<PersistedMessageDTO> messages = new ArrayList<>(List.of(persistedMessageDTO));
PersistedConversationDTO persistedConversationDTO = new PersistedConversationDTO(id, messages);
when(storage.get(id)).thenReturn(persistedConversationDTO);
Conversation conversation = conversationManager.getConversation(id);
@@ -142,8 +142,8 @@ public class ConversationManagerImplTest {
@Test
public void getConversationsReturnsAllFromStorage() {
when(storage.getKeys()).thenReturn(Set.of("conv1", "conv2"));
when(storage.get("conv1")).thenReturn(new ConversationDTO(new ArrayList<>()));
when(storage.get("conv2")).thenReturn(new ConversationDTO(new ArrayList<>()));
when(storage.get("conv1")).thenReturn(new PersistedConversationDTO("conv1", new ArrayList<>()));
when(storage.get("conv2")).thenReturn(new PersistedConversationDTO("conv2", new ArrayList<>()));
Collection<Conversation> conversations = conversationManager.getConversations();
@@ -168,7 +168,7 @@ public class ConversationManagerImplTest {
conversation.addMessage(ConversationRole.USER, "Hello");
verify(storage).put(eq(id), any(ConversationDTO.class));
verify(storage).put(eq(id), any(PersistedConversationDTO.class));
}
@Test
@@ -178,7 +178,7 @@ public class ConversationManagerImplTest {
conversation.addMessage(ConversationRole.USER, "Hello");
verify(storage, never()).put(eq(id), any(ConversationDTO.class));
verify(storage, never()).put(eq(id), any(PersistedConversationDTO.class));
}
@Test
@@ -205,7 +205,7 @@ public class ConversationManagerImplTest {
clearInvocations(storage, storageService); // clear stores from setting up conversation
conversation.removeSinceMessage(1);
verify(storage).put(eq(id), any(ConversationDTO.class));
verify(storage).put(eq(id), any(PersistedConversationDTO.class));
clearInvocations(storage, storageService);
conversation.removeSinceMessage(0);
@@ -225,6 +225,6 @@ public class ConversationManagerImplTest {
conversation.removeSinceMessage(3);
conversation.removeSinceMessage(0);
verify(storage, never()).put(eq(id), any(ConversationDTO.class));
verify(storage, never()).put(eq(id), any(PersistedConversationDTO.class));
}
}
bundles/org.openhab.core.voice/src/test/java/org/openhab/core/voice/internal/text/interpreter/StandardInterpreterTest.java
+11 -5
View File
@@ -22,6 +22,7 @@ import static org.openhab.core.voice.text.interpreter.rulebased.AbstractRuleBase
import static org.openhab.core.voice.text.interpreter.rulebased.AbstractRuleBasedInterpreter.IS_SILENT_CONFIGURATION;
import static org.openhab.core.voice.text.interpreter.rulebased.AbstractRuleBasedInterpreter.IS_TEMPLATE_CONFIGURATION;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
@@ -56,10 +57,13 @@ import org.openhab.core.library.types.UpDownType;
import org.openhab.core.types.CommandDescription;
import org.openhab.core.types.CommandOption;
import org.openhab.core.types.State;
import org.openhab.core.voice.DialogContext;
import org.openhab.core.voice.STTService;
import org.openhab.core.voice.TTSService;
import org.openhab.core.voice.text.InterpretationException;
import org.openhab.core.voice.text.InterpreterContext;
import org.openhab.core.voice.text.conversation.Conversation;
import org.openhab.core.voice.text.conversation.ConversationException;
import org.openhab.core.voice.text.conversation.ConversationRole;
/**
* Test the standard interpreter
@@ -124,7 +128,7 @@ public class StandardInterpreterTest {
}
@Test
public void noNameCollisionWhenDialogContext() throws InterpretationException {
public void noNameCollisionWhenLocationItem() throws InterpretationException, ConversationException {
var locationGroup = Mockito.spy(new GroupItem("livingroom"));
locationGroup.setLabel("Living room");
var computerItem = new SwitchItem("computer");
@@ -132,13 +136,15 @@ public class StandardInterpreterTest {
var computerItem2 = new SwitchItem("computer2");
computerItem2.setLabel("Computer");
when(locationGroup.getMembers()).thenReturn(Set.of(computerItem));
var dialogContext = new DialogContext(null, null, sttService, ttsService, null, List.of(), audioSource,
audioSink, Locale.ENGLISH, "", locationGroup.getName(), null, null, null, List.of());
List<Item> items = List.of(computerItem2, locationGroup, computerItem);
when(itemRegistryMock.getItems()).thenReturn(items);
Conversation conversation = new Conversation("test-conversation");
conversation.addMessage(ConversationRole.USER, "turn off computer");
InterpreterContext interpreterContext = new InterpreterContext(conversation, Collections.emptyList(),
locationGroup.getName());
// "computer" should only match the computerItem in the locationGroup
assertEquals(OK_RESPONSE, standardInterpreter.interpret(Locale.ENGLISH, "turn off computer", dialogContext));
assertEquals(OK_RESPONSE, standardInterpreter.interpret(Locale.ENGLISH, interpreterContext));
verify(eventPublisherMock, times(1))
.post(ItemEventFactory.createCommandEvent(computerItem.getName(), OnOffType.OFF));
}