Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[voicerss] Fix and enhance the external tool to prefill the cache #12155

Merged
merged 5 commits into from
Feb 2, 2022
Merged

[voicerss] Fix and enhance the external tool to prefill the cache #12155

merged 5 commits into from
Feb 2, 2022

Conversation

lolodomo
Copy link
Contributor

@lolodomo lolodomo commented Jan 29, 2022 •
edited
Loading

Make the tool independent of core openHAB

Fix the parsing of the command arguments.
Allow to provide optional audio codec and format.

Null annotations added on most of the classes

Signed-off-by: Laurent Garnier lg.hc@free.fr

@lolodomo lolodomo added bug An unexpected problem or unintended behavior of an add-on work in progress A PR that is not yet ready to be merged labels Jan 29, 2022
@lolodomo
Copy link
Contributor Author

lolodomo commented Jan 29, 2022 •
edited
Loading

To run this tool in a console command, you need to specify the classpath of 1 the voicerss jar and 2 the core.audio jar. This is even not enough as slf4j jar is still missing. I don't find this slf4j jar in a standard user installation.
The only solution I found was to comment all usage of Logger in voicerss code (this is not part of the current PR) and then I am able to run successfully the tool in a console command.

Should I update the POM file to build a specific additional jar (for this tool) including all the necessary dependencies ?
Advice would be appreciated.

@lolodomo
Copy link
Contributor Author

lolodomo commented Jan 29, 2022 •
edited
Loading

I just realized the dependency of the cache tool with core openHAB was introduced by recently merged PR #11916 .
I will have to partially revert this PR to have the cache tool non depending on openHAB core.
But this will not solve the dependency with slf4j.

@lolodomo lolodomo force-pushed the voicerss_cache_tool branch 2 times, most recently from 9bea0de to 656b2bd Compare January 29, 2022 20:12
@lolodomo lolodomo removed the work in progress A PR that is not yet ready to be merged label Jan 29, 2022
@lolodomo
Copy link
Contributor Author

The 2 problems are solved:

  1. Dependency of the cache tool with openHAB core is removed
  2. Requirement of slf4j to use the cache tool is removed

The PR is ready for a review.

@lolodomo
[voicerss] Fix and enhance the external tool to prefill the cache
a621d4d 
Make the tool independent of core openHAB

Fix the parsing of the command arguments.
Allow to provide optional audio codec and format.

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo lolodomo force-pushed the voicerss_cache_tool branch from 656b2bd to a621d4d Compare January 29, 2022 20:27
@lolodomo
Null annotations added on most of the classes
58e03fe 
Signed-off-by: Laurent Garnier <lg.hc@free.fr>
jlaur
jlaur requested changes Jan 30, 2022
View reviewed changes
Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

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

Not a full review for now, just spotted something that might be worth checking.

...hab.voice.voicerss/src/main/java/org/openhab/voice/voicerss/internal/VoiceRSSTTSService.java Show resolved Hide resolved
...hab.voice.voicerss/src/main/java/org/openhab/voice/voicerss/internal/VoiceRSSTTSService.java Show resolved Hide resolved
@lolodomo
Simplified doc about classpath
b6dd7f8 
Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo
Suppress obsolete mention of open issue in doc
0220a5c 
Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo
Review comment: fix missing break in switch
954783f 
Signed-off-by: Laurent Garnier <lg.hc@free.fr>
jlaur
jlaur approved these changes Feb 2, 2022
View reviewed changes
Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

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

LGTM

@jlaur jlaur merged commit 624efab into openhab:main Feb 2, 2022
@jlaur jlaur added this to the 3.3 milestone Feb 2, 2022
@lolodomo lolodomo deleted the voicerss_cache_tool branch February 2, 2022 17:40
NickWaterton pushed a commit to NickWaterton/openhab-addons that referenced this pull request Apr 27, 2022
@lolodomo @NickWaterton
[voicerss] Fix and enhance the external tool to prefill the cache (op…
c4b6b23 
…enhab#12155)

* [voicerss] Fix and enhance the external tool to prefill the cache
* Make the tool independent of core openHAB
* Fix the parsing of the command arguments.
* Allow to provide optional audio codec and format.
* Null annotations added on most of the classes
* Simplified doc about classpath
* Suppress obsolete mention of open issue in doc

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
Signed-off-by: Nick Waterton <n.waterton@outlook.com>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Jun 29, 2022
@lolodomo @nemerdaud
[voicerss] Fix and enhance the external tool to prefill the cache (op…
8fca2dd 
…enhab#12155)

* [voicerss] Fix and enhance the external tool to prefill the cache
* Make the tool independent of core openHAB
* Fix the parsing of the command arguments.
* Allow to provide optional audio codec and format.
* Null annotations added on most of the classes
* Simplified doc about classpath
* Suppress obsolete mention of open issue in doc

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
@lolodomo @andan67
[voicerss] Fix and enhance the external tool to prefill the cache (op…
216d82f 
…enhab#12155)

* [voicerss] Fix and enhance the external tool to prefill the cache
* Make the tool independent of core openHAB
* Fix the parsing of the command arguments.
* Allow to provide optional audio codec and format.
* Null annotations added on most of the classes
* Simplified doc about classpath
* Suppress obsolete mention of open issue in doc

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Nov 12, 2022
@lolodomo @andrasU
[voicerss] Fix and enhance the external tool to prefill the cache (op…
87e527f 
…enhab#12155)

* [voicerss] Fix and enhance the external tool to prefill the cache
* Make the tool independent of core openHAB
* Fix the parsing of the command arguments.
* Allow to provide optional audio codec and format.
* Null annotations added on most of the classes
* Simplified doc about classpath
* Suppress obsolete mention of open issue in doc

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Feb 23, 2023
@lolodomo @psmedley
[voicerss] Fix and enhance the external tool to prefill the cache (op…
b6eed2c 
…enhab#12155)

* [voicerss] Fix and enhance the external tool to prefill the cache
* Make the tool independent of core openHAB
* Fix the parsing of the command arguments.
* Allow to provide optional audio codec and format.
* Null annotations added on most of the classes
* Simplified doc about classpath
* Suppress obsolete mention of open issue in doc

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlaur jlaur jlaur approved these changes

@JochenHiller JochenHiller Awaiting requested review from JochenHiller

Assignees
No one assigned
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Milestone
3.3
Development

Successfully merging this pull request may close these issues.
2 participants
@lolodomo @jlaur