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

Detect alternate drive DNS install locations and versions #242 #243

Merged
merged 10 commits into from
Jul 19, 2018

Conversation

LexiconCode
Copy link
Member

@LexiconCode LexiconCode commented Jun 30, 2018

The premise is the search for DNS across all drives and to check DNS version.

@LexiconCode LexiconCode added WIP An work in progress Enhancement Enhancement of an existing feature labels Jun 30, 2018
@LexiconCode LexiconCode self-assigned this Jun 30, 2018
@LexiconCode LexiconCode added Complete Pull request is complete and tested as defined by Contributor Needs Review A pull request needs a code review. and removed WIP An work in progress labels Jul 1, 2018
if os.path.isfile(ExePath):
return ExePath
else:
print ' Dragon Naturally Speaking' + str(Version) + ' is not supported by Castor. Only versions 13 and above are supported. Purchase Dragon Naturally Speaking 13 or above'
Copy link

Choose a reason for hiding this comment

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

Caster.

@LexiconCode LexiconCode added WIP An work in progress and removed Complete Pull request is complete and tested as defined by Contributor labels Jul 4, 2018
@LexiconCode
Copy link
Member Author

LexiconCode commented Jul 4, 2018

@masisley Nice catch, thanks for reviewing! Because we use wmi and it takes time to search the registry of the def _find_natspeak():takes up to 15 seconds to complete. I don't want to add 15 seconds to the start up time of Caster.

I'll add a create something like def _verify_natspeak_path (): which will check if the current "ENGINE_PATH" is valid if not def _find_natspeak(): will be called to find the path.

@LexiconCode LexiconCode added Complete Pull request is complete and tested as defined by Contributor and removed WIP An work in progress labels Jul 12, 2018
@LexiconCode
Copy link
Member Author

This should be feature complete and robust. Your feedback would be appreciated!

There are at least two ways you can help test.

  1. Change "ENGINE_PATH": "C:/Program Files (x86)/Nuance/NaturallySpeaking15/Program/natspeak.exe", in settings.json to an invalid path. Restart Dragon
  2. Backup then delete settings.json regenerating default configuration. Restart Dragon

@BazookaMusic
Copy link
Contributor

Will tested by tomorrow and report.

@BazookaMusic
Copy link
Contributor

So:

  1. This fails with an error:
Searching Windows Registry For DNS...
Search Complete.
Setting DNS path to C:/Program Files (x86)/Nuance/NaturallySpeaking15/Program/natspeak.exe


ValueError('Extra data: line 85 column 2 - line 85 column 3 (char 3635 - 3636)',) while loading settings file: C:\NatLink\NatLink\MacroSystem\caster\bin\data\settings.json


(<type 'exceptions.ValueError'>, ValueError('Extra data: line 85 column 2 - line 85 column 3 (char 3635 - 3636)',), <traceback object at 0x0E9A3DC8>)

the engine path gets set to empty string. Restarting Dragon fixes this, the proper path is found and set,but the reboot command doesn't work until the path is set because it depends on it (extremely annoying).
2. Works as expected.

Copy link
Contributor

@BazookaMusic BazookaMusic left a comment

Choose a reason for hiding this comment

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

These changes should fix the engine path being replaced by empty string.

@@ -71,7 +106,7 @@ def _find_natspeak():
# EXECUTABLES
"DEFAULT_BROWSER_PATH": "C:/Program Files (x86)/Mozilla Firefox/firefox.exe",
"DOUGLAS_PATH": BASE_PATH + "/asynch/mouse/grids.py",
"ENGINE_PATH": _find_natspeak(),
"ENGINE_PATH": _validate_natspeak(),
Copy link
Contributor

@BazookaMusic BazookaMusic Jul 17, 2018

Choose a reason for hiding this comment

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

Validate should return the exe path , instead it returns empty string as noted in the other comment.

print "Setting DNS path to " + exe_path
except Exception as e:
print "Error saving json file " + str(e) + _SETTINGS_PATH
return ""
Copy link
Contributor

@BazookaMusic BazookaMusic Jul 17, 2018

Choose a reason for hiding this comment

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

The functions should always return exe_path so that empty string is not set as the default engine path.

@LexiconCode LexiconCode added this to the v6 milestone Jul 17, 2018
@LexiconCode
Copy link
Member Author

LexiconCode commented Jul 17, 2018

I appreciate the feedback! Showing my ignorance here. What were the steps to reproduce this?

the engine path gets set to empty string. Restarting Dragon fixes this, the proper path is found and set,but the reboot command doesn't work until the path is set because it depends on it (extremely annoying).

My intent was the only condition when an empty string is set when _find_natspeak() cannot find DNS path/supported DNS or _validate_natspeak() has an error loading/saving settings.json.

An empty string "ENGINE_PATH":"" shouldn't cause failure to load settings.json. by breaking json formatting. I'll defer to your advice but I'm trying to understand my failure of logic.

In addition, could you upload settings.json that contains the extra data?

Edit:

One false assumption I made.

  1. Load Caster with a valid ENGINE_PATH.
  2. Edit settings.json ENGINE_PATH to an invalid path while castor is still loaded.
  3. Execute the command "Reboot Dragon"

"Reboot Dragon" still executes as expected despite having an invalid ENGINE_PATH in settings.json. Therefore the valid path is being pulled from memory when castor initialized settings.json before editing with invalid path.

My assumption was that "settings.json" was read from file when the reboot command was executed. Which is partially correct in terms of what's displayed in command prompt but not the path set ENGINE_PATH in memory.

I can understand ENGINE_PATH has an empty string that the reboot Dragon command does not execute as expected. But that still doesn't explain your error.

In your I think in the scenario you described

  1. An empty string was set settings.json after a previous error during the first time loaded
  2. The functions correctly updated in settings.json.
  3. Yet in memory ENGINE_PATH still had an empty string.

In that case reboot Dragon should fail that doesn't seem to be related to the error as you described.

From my understanding which is very limited and ENGINE_PATH:"" or ENGINE_PATH: invalid string/path should be equivalent in this context. Ex. ENGINE_PATH:"" is different than ENGINE_PATH: null A null value is invalid in .Json syntax. Which would lead to coercing to Unicode: need string or buffer, NoneType found

You can simulate this by changing settings.py deleting ENGINE_PATH:"some path" to ENGINE_PATH:"" Then restarting or starting DNS which doesn't replicate the error. Bottom line is I'm missing something possibly related to the order of execution of code?

@BazookaMusic
Copy link
Contributor

BazookaMusic commented Jul 18, 2018

Destroy the path and reboot:
"ENGINE_PATH": "C:/Programhello Files (x86)/Nuance/NaturallySpeaking15/Program/natspeak.exe",
the new path is:
"ENGINE_PATH": "",

The full error message because I missed an important line:

Search Complete.
Setting DNS path to C:/Program Files (x86)/Nuance/NaturallySpeaking15/Program/natspeak.exe


ValueError('Extra data: line 85 column 3 - line 86 column 2 (char 3636 - 3639)',) while loading settings file: C:\NatLink\NatLink\MacroSystem\caster\bin\data\settings.json


(<type 'exceptions.ValueError'>, ValueError('Extra data: line 85 column 3 - line 86 column 2 (char 3636 - 3639)',), <traceback object at 0x0EB74DA0>)
Default settings values added: 7

The reboot Dragon command opens a file Explorer window to the correct path but doesn't actually run NatSpeak.exe. This could be unrelated to your additions so it could be ignored if the latter problem is fixed.

After restarting Dragon again:

Search Complete.
Setting DNS path to C:/Program Files (x86)/Nuance/NaturallySpeaking15/Program/natspeak.exe

to see what happens in the first case with the loading, I put the _save call from the _init function in a comment. Your function somehow adds extra curly braces to the json file. This is why it fails to load and instead loads the default values which had the engine path set as empty string. The settings file is generated again so everything works after that.

@LexiconCode
Copy link
Member Author

LexiconCode commented Jul 18, 2018

Thanks for explaining.

So to reproduce

Destroy the path and reboot:
"ENGINE_PATH": "C:/Program Files (x86)/Nuance/NaturallySpeaking15/Program/natspeak.exe",
the new path is:
"ENGINE_PATH": "",

I can't reproduce the error.

Can you reproduce it by?

  1. Backing up then deleting settings.json
  2. Letting the default settings.json generate with the correct path
  3. Edit the path and reboot:

"ENGINE_PATH": "C:/Program Files (x86)/Nuance/NaturallySpeaking15/Program/natspeak.exe",
the new path is:
"ENGINE_PATH": "",

  1. Is the error reproduced? If it is reproduced then it's definitely something wrong with the code if not I need to have your original settings.json.

@LexiconCode
Copy link
Member Author

As for

The reboot Dragon command opens a file Explorer window to the correct path but doesn't actually run NatSpeak.exe.

Castor initializes loading settings.json into memory <- containing invalid engine path
_validate_natspeak() finds an invalid engine path and write a complete settings.json with the updated engine path.
Unfortunately the engine path still is invalid in memory and is not updated. reboot Dragon command opens a file Explorer window to the correct path because it reads from file not from memory to display path but the call is pulled from memory which is still invalid.

Check out def reboot(wsr=False): in utilities.py line 122

@BazookaMusic
Copy link
Contributor

I first deleted settings.json, rebooted and then destroyed the path and rebooted again.
File generated from your code(commented out default settings override):https://ufile.io/gxd8h
original settings(after default settings override):https://ufile.io/7trlo
Exact same error , but the proper engine path is set after your latest change. Notice the extra braces in the last 2 lines of the first file.

@BazookaMusic
Copy link
Contributor

BazookaMusic commented Jul 18, 2018

This could be something wonky but what seems to fix the problem is to use a "w" flag instead of "r+" when you open settings.json inside validate_NatSpeak. Instead of truncating the file and then writing it ,you are writing over existing characters.Since your new configuration is actually a smaller file than the one I create with extra noise in the path ,some extra brackets remain in the end. After testing with longer noise text, this seems to be the case as the extra characters end up in other positions.

@LexiconCode
Copy link
Member Author

LexiconCode commented Jul 18, 2018

Thank you for taking the time to test.

Feels like I'm banging my head against a brick wall. Fortunately it makes sense why I can't reproduce it up till now. My bad assumption was I thought it was overwriting the file. I really do dislike working with json in part because I'm new to utilizing it and because you can't update in place data.

I'm still flummoxed by the idea I can't reproduce it using. The default settings should be the same file size in terms lines of code.

Backing up then deleting settings.json
Letting the default settings.json generate with the correct path
Edit the path and reboot:

"ENGINE_PATH": "C:/Program Files (x86)/Nuance/NaturallySpeaking15/Program/natspeak.exe",
the new path is:
"ENGINE_PATH": "",

Regardless I believe you hit the nail on the head though with the root of the problem. I'll include "w" flag instead of "r+" in def _validate_natspeak()

@LexiconCode
Copy link
Member Author

Hopefully this is resolved and let me know. Thanks for your time to review and test!

@BazookaMusic
Copy link
Contributor

BazookaMusic commented Jul 19, 2018

Seems to work great now!

@LexiconCode LexiconCode merged commit bc867b9 into dictation-toolbox:develop Jul 19, 2018
@LexiconCode LexiconCode removed the Needs Review A pull request needs a code review. label Sep 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complete Pull request is complete and tested as defined by Contributor Enhancement Enhancement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants