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.

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

Fix SAPI error code mappings #46841

Merged
merged 6 commits into from
Jan 12, 2021
Merged

Conversation

danmoseley
Copy link
Member

@danmoseley danmoseley commented Jan 12, 2021

Fixes #46801

  1. Resource messages for SAPI error codes are looked up by index. I had broken this mapping because I didn't realize that whoever defined the SAPI error codes seemingly accidentally jumped from hex to decimal in two places and missed two blocks of 6 codes. Fixed this and added error messages for some SAPI codes that were never included (at the end - seemingly a mistake) and added a bunch of comments. As best I can tell, it's now correct.

  2. Disable the SpeechRecognizerTest when the platform doesn't support speech, or it's not initialized, or not installed, or not enabled somehow.

Running CI first with 2nd part disabled so I can verify the error messages are now correct.

@danmoseley
Copy link
Member Author

/azp run runtime-libraries-coreclr outerloop-windows

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@danmoseley
Copy link
Member Author

/azp run runtime-libraries-coreclr outerloop-windows

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@danmoseley
Copy link
Member Author

/azp run runtime-libraries-mono outerloop

@danmoseley
Copy link
Member Author

/azp run runtime-libraries-coreclr outerloop-windows

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@danmoseley
Copy link
Member Author

OK, validated by forcing the tests to fail that the messages are now appropriate:

Windows 81, Windows 7

 System.PlatformNotSupportedException : No recognizer is installed.
      Stack Trace:
        /_/src/libraries/System.Speech/src/Recognition/RecognizerBase.cs(455,0): at System.Speech.Recognition.RecognizerBase.Initialize(SapiRecognizer recognizer, Boolean inproc)

Windows 10 / remote desktop

     System.InvalidOperationException : No audio device is installed.
      Stack Trace:
        /_/src/libraries/System.Speech/src/Recognition/RecognizerBase.cs(455,0): at System.Speech.Recognition.RecognizerBase.Initialize(SapiRecognizer recognizer, Boolean inproc)

Now doing it for real.

@danmoseley danmoseley marked this pull request as ready for review January 12, 2021 05:27
@danmoseley
Copy link
Member Author

/azp run runtime-libraries-coreclr outerloop-windows

@danmoseley
Copy link
Member Author

/azp run runtime-libraries-mono outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@danmoseley
Copy link
Member Author

once this is merged I need to cherry pick it into #46729

@Wraith2
Copy link
Contributor

Wraith2 commented Jan 12, 2021

whoever defined the SAPI error codes seemingly accidentally jumped from hex to decimal in two places and missed two blocks of 6 codes.

I thought I heard a faint cry of "Why! why would you do that!" somewhere... 😁

Co-authored-by: Stephen Toub <stoub@microsoft.com>
@ghost
Copy link

ghost commented Jan 12, 2021

Hello @danmosemsft!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@danmoseley
Copy link
Member Author

Failures are unrelated libraries - I will take a look later.

@danmoseley danmoseley merged commit 430c07a into dotnet:master Jan 12, 2021
@danmoseley danmoseley deleted the fix.sapi.errors branch January 12, 2021 19:38
danmoseley added a commit to danmoseley/runtime that referenced this pull request Jan 12, 2021
* Fix PKT

* Fix SAPI errors

* Disable test as appropriate

* enable more tests to fail to get messages

* reenable tests

* Update src/libraries/System.Speech/tests/SpeechRecognizerTests.cs

Co-authored-by: Stephen Toub <stoub@microsoft.com>

Co-authored-by: Stephen Toub <stoub@microsoft.com>
Anipik added a commit that referenced this pull request Jan 14, 2021
* Port System.Speech to .NET Core (#45941)

* Initial sources, with banners

* Run the code formatter

* Fix hang in XUnit due to failing to complete all AsyncOperation-s

* Remove reflection over RegistryKey

* Add ref and packaging

* Add tests

* Add sln

* Fix CS1584

* Fix CA1823

* Fix CA1834

* Unnecessary suppressions

* Fix SA1028

* Fix CA1507

* Fix CA1810

* Fix CA1825

* Fix CA1825

* Unnecessary suppressions

* Fix CA1805

* Fix IDE0004

* Fix IDE0090

* Remove CAS

* Remove tabs and dead code

* Unnecessary suppressions

* Fix SA1212

* Fix SA1121

* Disable SA1129

* Fix SA1206

* Fix SA1518

* Fix SA1617

* Fix SA1001

* Fix CS0618

* Remove unnecessary comments

* Remove unnecessary whitespace

* Remove low value xml doc comments

* Unused usings

* dead files

* Remove CAS

* More junk

* Fix obvious original bug

* Remove/insert newlines

* Remove reference to old design document

* Fix spacing

* Fix typo name

* Fix file casing

* Remove dead code

* Add to compat pack

* Remove AppDomain etc

* Fix casing of .NET

* Remove low value XML docs

* Remove code that relies on compiling assemblies

* Fix inadvertently removed padding

* Use EDI to preserve stack when rethrowing

* Fix misaligned resource ID's to match sperror.h

* Skip SpeechRecognitionEngine tests if no installed recognizers

* Fix misformatted string bug

* Logging for CI error

* Fix NRE trying to map phonemes for voice for culture for which we do not have phoneme map

* Fix 153 spelling errors in comments using `Visual Studio Spell Checker`

* Remove extraneous file

* Fix spacing

* Fix project reference

* Reorder properties in csproj

* Change from netcoreapp2.0 to netcoreapp2.1

* Update src/libraries/System.Speech/pkg/System.Speech.pkgproj

Co-authored-by: Jose Perez Rodriguez <joperezr@microsoft.com>

* Add build error for targeting netcoreapp2.0

* Suppress new error during packaging testing

* Update System.Speech.targets

* Remove ref comments

* Update pkgproj

* Remove placeholder

Co-authored-by: Jose Perez Rodriguez <joperezr@microsoft.com>
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>

* Fix port

* Disable some Speech tests on Server Core

* Set PackageVersion in Directory.Build.props

* Change packageIndex back to 5.0.0

* Don't rev the platforms package unless we ship it

* Fix PKT

* Add to libraries-packages.proj

* Fix SAPI error code mappings (#46841)

* Fix PKT

* Fix SAPI errors

* Disable test as appropriate

* enable more tests to fail to get messages

* reenable tests

* Update src/libraries/System.Speech/tests/SpeechRecognizerTests.cs

Co-authored-by: Stephen Toub <stoub@microsoft.com>

Co-authored-by: Stephen Toub <stoub@microsoft.com>

Co-authored-by: Jose Perez Rodriguez <joperezr@microsoft.com>
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
Co-authored-by: Eric StJohn <ericstj@microsoft.com>
Co-authored-by: Stephen Toub <stoub@microsoft.com>
Co-authored-by: Anirudh Agnihotry <anirudhagnihotry098@gmail.com>
@ghost ghost locked as resolved and limited conversation to collaborators Feb 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System.InvalidOperationException : The word passed to the normalize interface cannot be normalized.
4 participants