From ce2784a46e9738690dc2031bf2eeaaaf419039e7 Mon Sep 17 00:00:00 2001 From: Dane Finlay Date: Wed, 27 Nov 2024 02:57:35 +1100 Subject: [PATCH] Make some changes to Natlink's message window (#214) * Fix Natlink message window pop-up menu initialization Re: #184. The message window's pop-up menu is now loaded from the executable resource data. It is easier to change now. * Add an Exit item to the Natlink message window's File sub-menu Re: #213. This item is grayed out (disabled) by default, for now. I've added a separator between it and the File>Reload item. * Adjust File>Exit menu item changes to do things the C++/Windows way Re: #184. That is, use boolean flags and a dedicated user window message to convey updates to the output window. This makes it easier to update the pop-up menu later down the line, if desired, by adding and hand- ling new flags. * Add a new function for controlling Natlink's message window Re: #213. The new function is setMessageWindow(). I have documented it in the NatlinkSource/natlink.txt file. It is now necessary to call this function with a Python callback to enable the message window. The default callback will soon reside in the natlinkcore code and be set from appsupp.cpp. Since it is related, this changeset includes modifications to the message window's File>Reload logic, re: #28. The default callback will do a narrower reloading of user modules. * Fix global callbacks that fail without thread safety This changes the CDragonCode::makeCallback() method to hold Python's GIL regardless of whether Natlink thread safety is enabled. A crash occurs if this is not done. * Adjust and fix the message window changes a little Re: #213. * Update appsupp - Show the message window before natConnect(). - Remove the now unused reloadPython() method. * Fix some indentation issues in two files * Fix a problem with setMessageWindow() * Stop releasing natlink objects on IDD_RELOAD This can cause inconsistent state in user/library code. --------- --- NatlinkSource/COM/appsupp.cpp | 30 +++---- NatlinkSource/COM/appsupp.h | 3 - NatlinkSource/DragonCode.cpp | 137 ++++++++++++++++++++++---------- NatlinkSource/DragonCode.h | 5 ++ NatlinkSource/MessageWindow.cpp | 83 ++++++++++++++++--- NatlinkSource/MessageWindow.h | 5 +- NatlinkSource/Resource.h | 3 +- NatlinkSource/natlink.rc.in | 4 +- NatlinkSource/natlink.txt | 64 ++++++++++----- NatlinkSource/pythwrap.cpp | 32 ++++++++ 10 files changed, 268 insertions(+), 98 deletions(-) diff --git a/NatlinkSource/COM/appsupp.cpp b/NatlinkSource/COM/appsupp.cpp index 7133fec0..ea112669 100644 --- a/NatlinkSource/COM/appsupp.cpp +++ b/NatlinkSource/COM/appsupp.cpp @@ -279,6 +279,10 @@ std::string DoPyConfig(void) { STDMETHODIMP CDgnAppSupport::Register( IServiceProvider * pIDgnSite ) { OutputDebugString(TEXT("CDgnAppSupport::Register")); + + // TODO check here with C++ if Natlink is enabled for the current + // windows user. If not, return S_OK early + // load and initialize the Python system std::string init_error = DoPyConfig(); Py_Initialize(); @@ -287,17 +291,21 @@ STDMETHODIMP CDgnAppSupport::Register( IServiceProvider * pIDgnSite ) m_pDragCode = initModule(); m_pDragCode->setAppClass( this ); + // start the message window, without a callback and with all menu items + // disabled, in order to use displayText() + // this is fine, the callback and menu will be set up later + m_pDragCode->setMessageWindow( NULL, 0x4 ); + // simulate calling natlink.natConnect() except share the site object BOOL bSuccess = m_pDragCode->natConnect( pIDgnSite ); if( !bSuccess ) { OutputDebugString( TEXT( "NatLink: failed to initialize NatSpeak interfaces") ); - m_pDragCode->displayText( "Failed to initialize NatSpeak interfaces\r\n", TRUE ); // TODO: bug? won't show + m_pDragCode->displayText( "Failed to initialize NatSpeak interfaces\r\n", TRUE ); return S_OK; } - - // only now do we have the window to show info and possible error messages from before - // Python init + + // show info and error messages DisplayVersions(m_pDragCode); if ( !init_error.empty()) { OutputDebugStringA(init_error.c_str() ); @@ -418,17 +426,3 @@ STDMETHODIMP CDgnAppSupport::EndProcess( DWORD dwProcessID ) { return S_OK; } - -//--------------------------------------------------------------------------- -// This utility function reloads the Python interpreter. It is called from -// the display window menu and is useful for debugging during development of -// natlinkmain and natlinkutils. In normal use, we do not need to reload the -// Python interpreter. - -void CDgnAppSupport::reloadPython() -{ - // finalize the Python interpreter - OutputDebugString( TEXT( "CDgnAppSupport::reloadPython" ) ); - - PyImport_ReloadModule(m_pNatlinkModule); -} diff --git a/NatlinkSource/COM/appsupp.h b/NatlinkSource/COM/appsupp.h index 63370885..456e424c 100644 --- a/NatlinkSource/COM/appsupp.h +++ b/NatlinkSource/COM/appsupp.h @@ -27,9 +27,6 @@ class ATL_NO_VTABLE CDgnAppSupport : CDgnAppSupport(); ~CDgnAppSupport(); - // call this function to re-initialize the Python interface - void reloadPython(); - DECLARE_REGISTRY_RESOURCEID(IDR_APPSUPP) DECLARE_NOT_AGGREGATABLE(CDgnAppSupport) diff --git a/NatlinkSource/DragonCode.cpp b/NatlinkSource/DragonCode.cpp index 51cf5a45..2152b1ea 100644 --- a/NatlinkSource/DragonCode.cpp +++ b/NatlinkSource/DragonCode.cpp @@ -1004,6 +1004,9 @@ void CDragonCode::removeDictObj(CDictationObject * pDictObj ) void CDragonCode::makeCallback( PyObject *pFunc, PyObject *pArgs ) { + // This procedure requires Python's GIL to be held. + PyGILState_STATE gstate = PyGILState_Ensure(); + m_nCallbackDepth += 1; PyObject * pRetn = PyEval_CallObject( pFunc, pArgs ); m_nCallbackDepth -= 1; @@ -1032,6 +1035,8 @@ void CDragonCode::makeCallback( PyObject *pFunc, PyObject *pArgs ) onAttribChanged( DGNSRAC_MICSTATE ); } } + + PyGILState_Release( gstate ); } //--------------------------------------------------------------------------- @@ -1203,36 +1208,56 @@ void CDragonCode::onTimer() void CDragonCode::onMenuCommand( WPARAM wParam ) { - if( LOWORD(wParam) == IDD_RELOAD ) + WORD nMenuItem = LOWORD( wParam ); + switch ( nMenuItem ) { - - // currently we do not support this operation if we are using thread - // support because I have not worked through the issues about what - // to do about the thread state - if( m_pThreadState ) - { - return; - } + case IDD_RELOAD: + // note by DF: + // PyWin32, a library we all depend on, does not support + // interpreter reinitialization. Please see the following + // page for more information: + // https://mail.python.org/pipermail/python-win32/2013-January/012671.html + // + // With this limitation in mind, I have adjusted Natlink's + // reload mechanism for narrower code-reload. // reload the Python subsystem displayText( "Reloading Python subsystem...\r\n", FALSE, FALSE ); - // Although we do not really care about the Python reference count - // we do want to reset the callbacks so we do not make a call into - // an obselete intrepreter. + // reset the callbacks set from Python setChangeCallback( Py_None ); setBeginCallback( Py_None ); setTimerCallback( Py_None ); setTrayIcon( "", "", Py_None ); - // We call this because the reinitialization will not free up the - // python objects. Note that we do free the COM objects but we do - // not free the Python objects. This means that we will have a - // minor memory leak but no object leaks (which would prevent - // shutdown of NatSpeak). - releaseObjects(); + // note: we do not release objects here any more because it + // may result in inconsistent state in user/library code + //releaseObjects(); + + break; + + case IDD_EXIT: + break; + + default: + return; + } - m_pAppClass->reloadPython(); + // invoke the message window callback, if one is set + if( m_pMessageWindowCallback ) + { + makeCallback( + m_pMessageWindowCallback, + Py_BuildValue( "(i)", nMenuItem ) ); + } + + // do any post-callback work + switch ( nMenuItem ) + { + case IDD_EXIT: + // kill the message window + setMessageWindow( Py_None ); + break; } } @@ -1536,16 +1561,10 @@ BOOL CDragonCode::initSecondWindow() return FALSE; } - // we store out class pointer in the window's extra data field so we + // we store our class pointer in the window's extra data field so we // get called back when a menu message occurs SetWindowLong( m_hMsgWnd, 0, (LONG)this ); - // tell the thread about out message window - if( m_pSecdThrd ) - { - m_pSecdThrd->setMsgWnd( m_hMsgWnd ); - } - return TRUE; } @@ -1687,15 +1706,6 @@ BOOL CDragonCode::natConnect( IServiceProvider * pIDgnSite, BOOL bUseThreads ) } #endif - // here we start the second thread for displaying messages; we only need - // this when we are called as a compatibility module - - if( pIDgnSite != NULL ) - { - OutputDebugString(L"CDragonCode::natConnect new MessageWindow"); - m_pSecdThrd = new MessageWindow(); - } - // Connect to NatSpeak if( !initGetSiteObject( pIDgnSite ) ) @@ -1885,12 +1895,8 @@ BOOL CDragonCode::natDisconnect() m_pIDgnSRTraining = NULL; m_pISRCentral = NULL; - // shutdown the second thread - if( m_pSecdThrd ) - { - delete m_pSecdThrd; - m_pSecdThrd = NULL; - } + // kill the message window + setMessageWindow( Py_None ); // destroy our hidden window if( m_hMsgWnd ) @@ -3696,6 +3702,55 @@ BOOL CDragonCode::setTrayIcon( return TRUE; } +//--------------------------------------------------------------------------- + +BOOL CDragonCode::setMessageWindow( + PyObject * pCallback, DWORD dwFlags ) +{ + if( pCallback == Py_None ) + { + Py_XDECREF( m_pMessageWindowCallback ); + m_pMessageWindowCallback = NULL; + + // shutdown the second thread + if( m_pSecdThrd ) + { + delete m_pSecdThrd; + m_pSecdThrd = NULL; + } + } + else + { + // ignore this check if the menu is to be disabled + if( !(dwFlags & 0x04) ) + { + NOTBEFORE_INIT( "setMessageWindow" ); + } + Py_XINCREF( pCallback ); + Py_XDECREF( m_pMessageWindowCallback ); + m_pMessageWindowCallback = pCallback; + + // start the second thread for displaying messages + if( !m_pSecdThrd ) + { + m_pSecdThrd = new MessageWindow( dwFlags ); + } + + // otherwise, update the window + else + { + m_pSecdThrd->updateWindow( dwFlags ); + } + + // tell the second thread about our message window + m_pSecdThrd->setMsgWnd( m_hMsgWnd ); + } + + return TRUE; +} + +//--------------------------------------------------------------------------- + void CDragonCode::logCookie( const char * pText, QWORD qCookie ) { DWORD *pCookie = (DWORD*)&qCookie; diff --git a/NatlinkSource/DragonCode.h b/NatlinkSource/DragonCode.h index 38102b73..230073b4 100644 --- a/NatlinkSource/DragonCode.h +++ b/NatlinkSource/DragonCode.h @@ -48,6 +48,7 @@ class CDragonCode m_pszLogFile = NULL; m_bHasTrayIcon = FALSE; m_pTrayIconCallback = NULL; + m_pMessageWindowCallback = NULL; m_pMessageStack = NULL; m_pIDgnSSvcOutputEventA=0; m_pIDgnSSvcOutputEvent=0; @@ -89,6 +90,7 @@ class CDragonCode BOOL deleteWord( char * wordName ); BOOL setWordInfo( char * wordName, DWORD wordInfo ); BOOL setTrayIcon( char * iconName, char * toolTip, PyObject * pCallback ); + BOOL setMessageWindow( PyObject * pCallback, DWORD dwflags = 0 ); PyObject * getCurrentModule(); PyObject * getCurrentUser(); @@ -267,6 +269,9 @@ class CDragonCode BOOL m_bHasTrayIcon; PyObject *m_pTrayIconCallback; + // set when the message window is started + PyObject * m_pMessageWindowCallback; + // This is what we call when we are ready to recume recognition void doPausedProcessing( QWORD dwCookie ); diff --git a/NatlinkSource/MessageWindow.cpp b/NatlinkSource/MessageWindow.cpp index 003adba0..dd4223fd 100644 --- a/NatlinkSource/MessageWindow.cpp +++ b/NatlinkSource/MessageWindow.cpp @@ -30,6 +30,14 @@ // thread. #define WM_EXITTHREAD (WM_USER+363) +// Send this message to the output window to update it as specified. +#define WM_UPDATEWINDOW (WM_USER+364) + +// WPARAM flags for the WM_UPDATEWINDOW message. +#define IDR_MENU_ENABLE_IDD_EXIT 0x01 +#define IDR_MENU_DISABLE_IDD_RELOAD 0x02 +#define IDR_MENU_DISABLE 0x04 + // The color for error messages #define DARKRED RGB( 128, 0, 0 ) @@ -47,22 +55,60 @@ INT_PTR CALLBACK dialogProc( switch( msg ) { - case WM_CREATE: + case WM_CREATE: + return TRUE; - return TRUE; case WM_INITDIALOG: - HMENU hMenu, hSubMenu; + // save a pointer to the message window for later + s_pSecdThrd = (MessageWindow *)lParam; + + // hide the window to start with + ShowWindow( hWnd, SW_HIDE ); + return TRUE; + + case WM_UPDATEWINDOW: + HINSTANCE hInstance; + HMENU hMenu, hSubMenu; + UINT uFlags, uIDItem; - hMenu = CreateMenu(); - hSubMenu = CreatePopupMenu(); - AppendMenu(hSubMenu, MF_STRING, IDD_RELOAD, L"&Reload"); - AppendMenu(hMenu, MF_STRING | MF_POPUP, (UINT)hSubMenu, L"&File"); + // destroy the current menu + hMenu = GetMenu( hWnd ); + if( hMenu ) + { + DestroyMenu( hMenu ); + } + // load the pop-up menu + hInstance = _Module.GetModuleInstance(); + hMenu = LoadMenu( hInstance, MAKEINTRESOURCE( IDR_MENU ) ); + hSubMenu = GetSubMenu( hMenu, 0 ); - SetMenu(hWnd, hMenu); - s_pSecdThrd = (MessageWindow *)lParam; - ShowWindow( hWnd, SW_HIDE ); + // disable all menu items, if specified + if( wParam & IDR_MENU_DISABLE ) + { + wParam &= ~IDR_MENU_ENABLE_IDD_EXIT; + wParam |= IDR_MENU_DISABLE_IDD_RELOAD; + } + + // enable the exit sub-menu item, if appropriate + if( wParam & IDR_MENU_ENABLE_IDD_EXIT ) + { + uFlags = MF_BYCOMMAND | MF_ENABLED; + uIDItem = IDD_EXIT; + EnableMenuItem( hSubMenu, uIDItem, uFlags ); + } + + // disable the reload sub-menu item, if appropriate + if( wParam & IDR_MENU_DISABLE_IDD_RELOAD ) + { + uFlags = MF_BYCOMMAND | MF_GRAYED; + uIDItem = IDD_RELOAD; + EnableMenuItem( hSubMenu, uIDItem, uFlags ); + } + + // assign the modified menu to the window + SetMenu( hWnd, hMenu ); return TRUE; case WM_EXITTHREAD: @@ -95,7 +141,7 @@ INT_PTR CALLBACK dialogProc( HIWORD(lParam), // height of client area TRUE); // repaint window } - return TRUE; + return TRUE; case WM_SHOWTEXT: { @@ -187,7 +233,7 @@ DWORD CALLBACK threadMain( void * pArg ) //--------------------------------------------------------------------------- -MessageWindow::MessageWindow() +MessageWindow::MessageWindow( DWORD dwFlags ) { // create the thread we use to display messages; we use an event to make // sure that the thread has started before continuing @@ -214,6 +260,9 @@ MessageWindow::MessageWindow() CloseHandle( m_hEvent ); m_hEvent = NULL; } + + // update the window with the specified flags + updateWindow( dwFlags ); } //--------------------------------------------------------------------------- @@ -244,3 +293,13 @@ void MessageWindow::displayText(const char * pszText, BOOL bError ) PostMessage( m_hOutWnd, WM_SHOWTEXT, bError, (LPARAM)pszCopy ); } } + +//--------------------------------------------------------------------------- + +void MessageWindow::updateWindow( DWORD dwFlags ) +{ + if( m_hOutWnd ) + { + PostMessage( m_hOutWnd, WM_UPDATEWINDOW, dwFlags, 0 ); + } +} diff --git a/NatlinkSource/MessageWindow.h b/NatlinkSource/MessageWindow.h index 6a736382..b689ccfb 100644 --- a/NatlinkSource/MessageWindow.h +++ b/NatlinkSource/MessageWindow.h @@ -11,7 +11,7 @@ class MessageWindow { public: // the constructor will create the thread - MessageWindow(); + MessageWindow( DWORD dwFlags ); // the destructor will terminate the thread ~MessageWindow(); @@ -23,6 +23,9 @@ class MessageWindow // WM_COMMAND messages received by the output window should be reposted void setMsgWnd( HWND hWnd ) { m_hMsgWnd = hWnd; } + // this will update the output window + void updateWindow( DWORD dwFlags ); + // these are called from the second thread itself void setOutWnd( HWND hWnd ) { m_hOutWnd = hWnd; } void signalEvent() { SetEvent( m_hEvent ); } diff --git a/NatlinkSource/Resource.h b/NatlinkSource/Resource.h index aa828be5..97293545 100644 --- a/NatlinkSource/Resource.h +++ b/NatlinkSource/Resource.h @@ -18,6 +18,7 @@ #define IDI_UP2 211 #define IDI_NODIR 212 #define IDD_RELOAD 32768 +#define IDD_EXIT 32769 // Next default values for new objects // @@ -25,7 +26,7 @@ #ifndef APSTUDIO_READONLY_SYMBOLS #define _APS_NO_MFC 1 #define _APS_NEXT_RESOURCE_VALUE 213 -#define _APS_NEXT_COMMAND_VALUE 32769 +#define _APS_NEXT_COMMAND_VALUE 32770 #define _APS_NEXT_CONTROL_VALUE 202 #define _APS_NEXT_SYMED_VALUE 102 #endif diff --git a/NatlinkSource/natlink.rc.in b/NatlinkSource/natlink.rc.in index 45d904cf..19f3aa6f 100644 --- a/NatlinkSource/natlink.rc.in +++ b/NatlinkSource/natlink.rc.in @@ -99,7 +99,7 @@ IDR_APPSUPP REGISTRY "appsupp.reg" IDD_STDOUT DIALOGEX 0, 25, 285, 135 STYLE DS_SETFONT | DS_MODALFRAME | WS_MINIMIZEBOX | WS_POPUP | WS_CAPTION | WS_SYSMENU | WS_THICKFRAME CAPTION "Messages from Natlink" -//MENU IDR_MENU +MENU IDR_MENU FONT 9, "Courier New", 0, 0, 0x0 BEGIN CONTROL "",IDC_RICHEDIT,"RICHEDIT",TCS_RAGGEDRIGHT | TCS_MULTISELECT | WS_BORDER | WS_VSCROLL | WS_TABSTOP,5,5,275,125 @@ -169,6 +169,8 @@ BEGIN POPUP "&File" BEGIN MENUITEM "&Reload Everything", IDD_RELOAD + MENUITEM SEPARATOR + MENUITEM "&Exit", IDD_EXIT, GRAYED END END diff --git a/NatlinkSource/natlink.txt b/NatlinkSource/natlink.txt index 603f0190..15ff093b 100644 --- a/NatlinkSource/natlink.txt +++ b/NatlinkSource/natlink.txt @@ -572,6 +572,28 @@ waitForSpeech( timeout ) timeout except that the message box is not displayed (the function only returns when the timeout elapses). +setMessageWindow( callback, flags ) + This function enables, disables and updates Natlink's message window. + + The function has one required parameter whose value should be a Python + function or None. If a function is passed for this parameter, it + should take one parameter which is the type of message window event: + idd_reload or idd_exit (each defined in natlinkutils) + + Passing a callback function enables the message window, passing None + disables it. + + The second parameter is optional. It is a combination of one or more + of the following flags: + 0x01 # enable the window's File>Exit menu item + 0x02 # disable the window's File>Reload menu item + 0x04 # disable the window's menu. Allows pre-natConnect() use + + Note: The idd_reload event occurs when the File>Reload menu item is + activated. Before this function's callback is invoked, Natlink will + unload all loaded grammars, destroy all existing ResObjs and DictObjs, + and clear all callbacks except the message window callback -- this one. + Inside natlink, each grammar is maintained as a separate object. To expose this structure, each grammar object will be exposed to Python as a class. The GrammarBase class defined in natlinkutils encapusulates and extends the @@ -586,18 +608,18 @@ class GramObj(): of the grammar in SAPI format (as a string). This call actually creates the associated COM objects. - If you set the optional allResults parameter to 1 then you will be - able to get a results object even when the recognition is not - specific to your grammar. + If you set the optional allResults parameter to 1 then you will be + able to get a results object even when the recognition is not + specific to your grammar. - If you optionally set hypothesis to 1 then partial recognition - hypothesises will be available during recognition using a hypothesis - callback (see setHypothesisCallback). + If you optionally set hypothesis to 1 then partial recognition + hypothesises will be available during recognition using a hypothesis + callback (see setHypothesisCallback). - The first DWORD of the SAPI binary grammar format defines the type - of grammar. Command grammars (like those created by GrammarBase - in natlinkutils) have a type of 0. Dictation grammars have a type - of 1. Not every member function of GramObj is supported by every + The first DWORD of the SAPI binary grammar format defines the type + of grammar. Command grammars (like those created by GrammarBase + in natlinkutils) have a type of 0. Dictation grammars have a type + of 1. Not every member function of GramObj is supported by every grammar type. Can raise InvalidWord if the grammar contains an invalid word. @@ -664,17 +686,17 @@ class GramObj(): system rejection then the first parameter passed to your callback function will be 'reject'. - setHypothesisCallback( pCallback ) - Call this to setup a callback function which will be called during - the middle of recognitions with the partial hypothesis of the - recognition in progress. You must have set the hypothesis - parameter on load or the callback will not be called. - - The callback routine is passed a list of words representing - the best recognition hypothesis so far. The frequency of callbacks - is determined by Dragon NaturallySpeaking. Any word in the - hypothesis is subject to chang ein the next callback or when the - recognition completes. + setHypothesisCallback( pCallback ) + Call this to setup a callback function which will be called during + the middle of recognitions with the partial hypothesis of the + recognition in progress. You must have set the hypothesis + parameter on load or the callback will not be called. + + The callback routine is passed a list of words representing + the best recognition hypothesis so far. The frequency of callbacks + is determined by Dragon NaturallySpeaking. Any word in the + hypothesis is subject to chang ein the next callback or when the + recognition completes. emptyList( listName ) This function removes all the words in a named list in the grammar. diff --git a/NatlinkSource/pythwrap.cpp b/NatlinkSource/pythwrap.cpp index bdd189ac..7c1ca580 100644 --- a/NatlinkSource/pythwrap.cpp +++ b/NatlinkSource/pythwrap.cpp @@ -1197,6 +1197,37 @@ natlink_setTrayIcon( PyObject *self, PyObject *args ) return Py_None; } +//--------------------------------------------------------------------------- +// natlink.setMessageWindow( callback, flags ) +// +// See natlink.txt for documentation. + +extern "C" static PyObject * +natlink_setMessageWindow( PyObject *self, PyObject *args ) +{ + PyObject * pFunc; + DWORD dwFlags = 0; + + if( !PyArg_ParseTuple( args, "O|i:setMessageWindow", &pFunc, &dwFlags) ) + { + return NULL; + } + + if( pFunc != Py_None && !PyCallable_Check( pFunc ) ) + { + PyErr_SetString( PyExc_TypeError, "first parameter must be callable" ); + return NULL; + } + + if( !cDragon.setMessageWindow( pFunc, dwFlags ) ) + { + return NULL; + } + + Py_INCREF( Py_None ); + return Py_None; +} + ///////////////////////////////////////////////////////////////////////////// //--------------------------------------------------------------------------- @@ -2317,6 +2348,7 @@ static struct PyMethodDef natlink_methods[] = { { "setWordInfo", natlink_setWordInfo, METH_VARARGS }, { "getWordProns", natlink_getWordProns, METH_VARARGS }, { "setTrayIcon", natlink_setTrayIcon, METH_VARARGS }, + { "setMessageWindow", natlink_setMessageWindow, METH_VARARGS }, { NULL } };