diff --git a/common/footprint_info.cpp b/common/footprint_info.cpp index 806e7f4b6a..b093e573e2 100644 --- a/common/footprint_info.cpp +++ b/common/footprint_info.cpp @@ -138,6 +138,22 @@ void FOOTPRINT_LIST::DisplayErrors( wxTopLevelWindow* aWindow ) } +wxString FOOTPRINT_LIST::GetErrorMessages() +{ + wxString messages; + + while( std::unique_ptr error = PopError() ) + { + if( !messages.IsEmpty() ) + messages += wxS( "\n" ); + + messages += error->Problem(); + } + + return messages; +} + + static FOOTPRINT_LIST* get_instance_from_id( KIWAY& aKiway, int aId ) { void* ptr = nullptr; diff --git a/common/libraries/library_manager.cpp b/common/libraries/library_manager.cpp index de357b735f..0e1cda391e 100644 --- a/common/libraries/library_manager.cpp +++ b/common/libraries/library_manager.cpp @@ -1040,6 +1040,26 @@ std::vector> LIBRARY_MANAGER_ADAPTER::GetLibrary } +wxString LIBRARY_MANAGER_ADAPTER::GetLibraryLoadErrors() const +{ + wxString errors; + + for( const auto& [nickname, status] : GetLibraryStatuses() ) + { + if( status.load_status == LOAD_STATUS::LOAD_ERROR && status.error ) + { + if( !errors.IsEmpty() ) + errors += wxS( "\n" ); + + errors += wxString::Format( _( "Library '%s': %s" ), + nickname, status.error->message ); + } + } + + return errors; +} + + void LIBRARY_MANAGER_ADAPTER::ReloadLibraryEntry( const wxString& aNickname, LIBRARY_TABLE_SCOPE aScope ) { diff --git a/common/pgm_base.cpp b/common/pgm_base.cpp index 6e692d17fb..8c615caf9e 100644 --- a/common/pgm_base.cpp +++ b/common/pgm_base.cpp @@ -69,6 +69,7 @@ #include #include +#include #include #ifdef KICAD_IPC_API @@ -947,6 +948,79 @@ void PGM_BASE::PreloadDesignBlockLibraries( KIWAY* aKiway ) } +void PGM_BASE::RegisterLibraryLoadStatusBar( KISTATUSBAR* aStatusBar ) +{ + std::lock_guard lock( m_libraryLoadStatusBarsMutex ); + + wxLogTrace( traceLibraries, "RegisterLibraryLoadStatusBar: statusBar=%p", aStatusBar ); + + if( std::find( m_libraryLoadStatusBars.begin(), m_libraryLoadStatusBars.end(), aStatusBar ) + == m_libraryLoadStatusBars.end() ) + { + m_libraryLoadStatusBars.push_back( aStatusBar ); + wxLogTrace( traceLibraries, " -> registered, total count=%zu", + m_libraryLoadStatusBars.size() ); + } + else + { + wxLogTrace( traceLibraries, " -> already registered" ); + } +} + + +void PGM_BASE::UnregisterLibraryLoadStatusBar( KISTATUSBAR* aStatusBar ) +{ + std::lock_guard lock( m_libraryLoadStatusBarsMutex ); + + wxLogTrace( traceLibraries, "UnregisterLibraryLoadStatusBar: statusBar=%p", aStatusBar ); + + m_libraryLoadStatusBars.erase( + std::remove( m_libraryLoadStatusBars.begin(), m_libraryLoadStatusBars.end(), + aStatusBar ), + m_libraryLoadStatusBars.end() ); + + wxLogTrace( traceLibraries, " -> remaining count=%zu", m_libraryLoadStatusBars.size() ); +} + + +void PGM_BASE::AddLibraryLoadMessages( const std::vector& aMessages ) +{ + wxLogTrace( traceLibraries, "AddLibraryLoadMessages: message_count=%zu", aMessages.size() ); + + if( aMessages.empty() ) + return; + + std::lock_guard lock( m_libraryLoadStatusBarsMutex ); + + wxLogTrace( traceLibraries, " -> registered status bars=%zu", + m_libraryLoadStatusBars.size() ); + + for( KISTATUSBAR* statusBar : m_libraryLoadStatusBars ) + { + if( statusBar ) + { + wxLogTrace( traceLibraries, " -> forwarding to statusBar=%p", statusBar ); + statusBar->AddLoadWarningMessages( aMessages ); + } + } +} + + +void PGM_BASE::ClearLibraryLoadMessages() +{ + std::lock_guard lock( m_libraryLoadStatusBarsMutex ); + + wxLogTrace( traceLibraries, "ClearLibraryLoadMessages: status bars=%zu", + m_libraryLoadStatusBars.size() ); + + for( KISTATUSBAR* statusBar : m_libraryLoadStatusBars ) + { + if( statusBar ) + statusBar->ClearLoadWarningMessages(); + } +} + + static PGM_BASE* process; diff --git a/common/string_utils.cpp b/common/string_utils.cpp index 00e021cfe3..5c1e200db8 100644 --- a/common/string_utils.cpp +++ b/common/string_utils.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -1781,3 +1782,33 @@ int SortVariantNames( const wxString& aLhs, const wxString& aRhs ) return StrNumCmp( aLhs, aRhs ); } + +std::vector ExtractLibraryLoadErrors( const wxString& aErrorString, int aSeverity ) +{ + std::vector messages; + + if( aErrorString.IsEmpty() ) + return messages; + + // Errors are separated by newlines. We want to keep: + // - Lines starting with "Library '" (library-level errors) + // - Lines containing "Expecting" (file error location) + // And strip: + // - Lines starting with "from " (internal code location info) + wxStringTokenizer tokenizer( aErrorString, wxS( "\n" ), wxTOKEN_STRTOK ); + + while( tokenizer.HasMoreTokens() ) + { + wxString line = tokenizer.GetNextToken(); + + // Skip internal code location lines (e.g., "from pcb_io_kicad_sexpr_parser.cpp : ...") + if( line.StartsWith( wxS( "from " ) ) ) + continue; + + if( line.StartsWith( wxS( "Library '" ) ) || line.Contains( wxS( "Expecting" ) ) ) + messages.push_back( { line, static_cast( aSeverity ) } ); + } + + return messages; +} + diff --git a/common/widgets/kistatusbar.cpp b/common/widgets/kistatusbar.cpp index 4281df344b..191dcba9c6 100644 --- a/common/widgets/kistatusbar.cpp +++ b/common/widgets/kistatusbar.cpp @@ -39,6 +39,7 @@ #include #include #include +#include #include @@ -316,42 +317,106 @@ void KISTATUSBAR::SetNotificationCount( int aCount ) void KISTATUSBAR::SetLoadWarningMessages( const wxString& aMessages ) { - m_loadWarningMessages.clear(); - - wxStringTokenizer tokenizer( aMessages, wxS( "\n" ), wxTOKEN_STRTOK ); - - while( tokenizer.HasMoreTokens() ) { - LOAD_MESSAGE msg; - msg.message = tokenizer.GetNextToken(); - msg.severity = RPT_SEVERITY_WARNING; // Default to warning for font substitutions - m_loadWarningMessages.push_back( msg ); - } + std::lock_guard lock( m_loadWarningMutex ); + m_loadWarningMessages.clear(); - if( m_warningButton ) - { - m_warningButton->Show( !m_loadWarningMessages.empty() ); + wxStringTokenizer tokenizer( aMessages, wxS( "\n" ), wxTOKEN_STRTOK ); - if( !m_loadWarningMessages.empty() ) + while( tokenizer.HasMoreTokens() ) { - m_warningButton->SetToolTip( - wxString::Format( _( "View %zu load message(s)" ), - m_loadWarningMessages.size() ) ); + LOAD_MESSAGE msg; + msg.message = tokenizer.GetNextToken(); + msg.severity = RPT_SEVERITY_WARNING; // Default to warning for font substitutions + m_loadWarningMessages.push_back( msg ); } - - Layout(); - Refresh(); } + + updateWarningUI(); +} + + +void KISTATUSBAR::AddLoadWarningMessages( const std::vector& aMessages ) +{ + wxLogTrace( traceLibraries, "KISTATUSBAR::AddLoadWarningMessages: this=%p, count=%zu", + this, aMessages.size() ); + + if( aMessages.empty() ) + return; + + { + std::lock_guard lock( m_loadWarningMutex ); + m_loadWarningMessages.insert( m_loadWarningMessages.end(), aMessages.begin(), aMessages.end() ); + wxLogTrace( traceLibraries, " -> total messages now=%zu", m_loadWarningMessages.size() ); + } + + // Update UI on main thread + wxLogTrace( traceLibraries, " -> calling CallAfter for updateWarningUI" ); + CallAfter( [this]() { updateWarningUI(); } ); +} + + +size_t KISTATUSBAR::GetLoadWarningCount() const +{ + std::lock_guard lock( m_loadWarningMutex ); + return m_loadWarningMessages.size(); +} + + +void KISTATUSBAR::updateWarningUI() +{ + wxLogTrace( traceLibraries, "KISTATUSBAR::updateWarningUI: this=%p, m_warningButton=%p", + this, m_warningButton ); + + if( !m_warningButton ) + { + wxLogTrace( traceLibraries, " -> no warning button, returning early" ); + return; + } + + size_t messageCount; + { + std::lock_guard lock( m_loadWarningMutex ); + messageCount = m_loadWarningMessages.size(); + } + + wxLogTrace( traceLibraries, " -> message count=%zu, showing button=%s", + messageCount, messageCount > 0 ? "true" : "false" ); + + m_warningButton->Show( messageCount > 0 ); + + if( messageCount > 0 ) + { + m_warningButton->SetToolTip( wxString::Format( _( "View %zu load message(s)" ), messageCount ) ); + + // Show count badge on the warning button + m_warningButton->SetShowBadge( true ); + wxString badgeText = messageCount > 99 + ? wxString( "99+" ) + : wxString::Format( wxS( "%zu" ), messageCount ); + m_warningButton->SetBadgeText( badgeText ); + + wxLogTrace( traceLibraries, " -> badge set to '%s'", badgeText ); + } + + Layout(); + Refresh(); + wxLogTrace( traceLibraries, " -> Layout and Refresh complete" ); } void KISTATUSBAR::ClearLoadWarningMessages() { - m_loadWarningMessages.clear(); + { + std::lock_guard lock( m_loadWarningMutex ); + m_loadWarningMessages.clear(); + } if( m_warningButton ) { m_warningButton->Hide(); + m_warningButton->SetShowBadge( false ); + m_warningButton->SetBadgeText( wxEmptyString ); Layout(); Refresh(); } @@ -360,12 +425,19 @@ void KISTATUSBAR::ClearLoadWarningMessages() void KISTATUSBAR::onLoadWarningsIconClick( wxCommandEvent& aEvent ) { - if( m_loadWarningMessages.empty() ) + // Copy messages under lock to avoid holding lock during modal dialog + std::vector messages; + { + std::lock_guard lock( m_loadWarningMutex ); + messages = m_loadWarningMessages; + } + + if( messages.empty() ) return; DIALOG_HTML_REPORTER dlg( GetParent(), wxID_ANY, _( "Load Messages" ) ); - for( const LOAD_MESSAGE& msg : m_loadWarningMessages ) + for( const LOAD_MESSAGE& msg : messages ) dlg.m_Reporter->Report( msg.message, msg.severity ); dlg.m_Reporter->Flush(); diff --git a/cvpcb/cvpcb_mainframe.cpp b/cvpcb/cvpcb_mainframe.cpp index 50009cc197..72f5a84a4e 100644 --- a/cvpcb/cvpcb_mainframe.cpp +++ b/cvpcb/cvpcb_mainframe.cpp @@ -43,6 +43,7 @@ #include #include #include +#include #include #include @@ -908,7 +909,10 @@ bool CVPCB_MAINFRAME::LoadFootprintFiles() m_FootprintsList->ReadFootprintFiles( adapter, nullptr, &progressReporter ); if( m_FootprintsList->GetErrorCount() ) - m_FootprintsList->DisplayErrors( this ); + { + if( KISTATUSBAR* statusBar = dynamic_cast( GetStatusBar() ) ) + statusBar->SetLoadWarningMessages( m_FootprintsList->GetErrorMessages() ); + } return true; } diff --git a/cvpcb/display_footprints_frame.cpp b/cvpcb/display_footprints_frame.cpp index 48686f2e7d..9048ffd6eb 100644 --- a/cvpcb/display_footprints_frame.cpp +++ b/cvpcb/display_footprints_frame.cpp @@ -308,7 +308,8 @@ FOOTPRINT* DISPLAY_FOOTPRINTS_FRAME::GetFootprint( const wxString& aFootprintNam } catch( const IO_ERROR& ioe ) { - DisplayErrorMessage( this, _( "Error loading footprint" ), ioe.What() ); + aReporter.Report( wxString::Format( _( "Error loading footprint: %s" ), ioe.What() ), + RPT_SEVERITY_ERROR ); return nullptr; } diff --git a/eeschema/cross-probing.cpp b/eeschema/cross-probing.cpp index d773d744ad..b9407fed5a 100644 --- a/eeschema/cross-probing.cpp +++ b/eeschema/cross-probing.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include #include @@ -1089,12 +1090,25 @@ void SCH_EDIT_FRAME::KiwayMailIn( KIWAY_EXPRESS& mail ) break; case MAIL_RELOAD_LIB: + { if( m_designBlocksPane && m_designBlocksPane->IsShown() ) { m_designBlocksPane->RefreshLibs(); SyncView(); } + + // Show any symbol library load errors in the status bar + if( KISTATUSBAR* statusBar = dynamic_cast( GetStatusBar() ) ) + { + SYMBOL_LIBRARY_ADAPTER* adapter = PROJECT_SCH::SymbolLibAdapter( &Prj() ); + wxString errors = adapter->GetLibraryLoadErrors(); + + if( !errors.IsEmpty() ) + statusBar->SetLoadWarningMessages( errors ); + } + break; + } default:; diff --git a/eeschema/eeschema.cpp b/eeschema/eeschema.cpp index 3e915c969d..209e8d608f 100644 --- a/eeschema/eeschema.cpp +++ b/eeschema/eeschema.cpp @@ -50,11 +50,14 @@ #include #include #include +#include #include #include #include +#include #include #include +#include #include #include @@ -482,6 +485,8 @@ void IFACE::PreloadLibraries( KIWAY* aKiway ) if( m_libraryPreloadInProgress.load() ) return; + Pgm().ClearLibraryLoadMessages(); + m_libraryPreloadBackgroundJob = Pgm().GetBackgroundJobMonitor().Create( _( "Loading Symbol Libraries" ) ); @@ -530,6 +535,26 @@ void IFACE::PreloadLibraries( KIWAY* aKiway ) adapter->BlockUntilLoaded(); + // Collect library load errors for async reporting + wxString errors = adapter->GetLibraryLoadErrors(); + + wxLogTrace( traceLibraries, "eeschema PreloadLibraries: errors.IsEmpty()=%d, length=%zu", + errors.IsEmpty(), errors.length() ); + + std::vector messages = + ExtractLibraryLoadErrors( errors, RPT_SEVERITY_ERROR ); + + if( !messages.empty() ) + { + wxLogTrace( traceLibraries, " -> collected %zu messages, calling AddLibraryLoadMessages", + messages.size() ); + Pgm().AddLibraryLoadMessages( messages ); + } + else + { + wxLogTrace( traceLibraries, " -> no errors from symbol libraries" ); + } + Pgm().GetBackgroundJobMonitor().Remove( m_libraryPreloadBackgroundJob ); m_libraryPreloadBackgroundJob.reset(); m_libraryPreloadInProgress.store( false ); diff --git a/eeschema/files-io.cpp b/eeschema/files-io.cpp index 6148e29e0d..3f14426541 100644 --- a/eeschema/files-io.cpp +++ b/eeschema/files-io.cpp @@ -641,7 +641,7 @@ bool SCH_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, in } // Update all symbol library links for all sheets. - schematic.UpdateSymbolLinks(); + schematic.UpdateSymbolLinks( &loadReporter ); m_infoBar->RemoveAllButtons(); m_infoBar->AddCloseButton(); diff --git a/eeschema/sch_io/altium/sch_io_altium.cpp b/eeschema/sch_io/altium/sch_io_altium.cpp index fe67b382e6..30aaff9c01 100644 --- a/eeschema/sch_io/altium/sch_io_altium.cpp +++ b/eeschema/sch_io/altium/sch_io_altium.cpp @@ -537,7 +537,7 @@ SCH_SHEET* SCH_IO_ALTIUM::LoadSchematicFile( const wxString& aFileName, SCHEMATI m_errorMessages.clear(); SCH_SCREENS allSheets( m_rootSheet ); - allSheets.UpdateSymbolLinks(); // Update all symbol library links for all sheets. + allSheets.UpdateSymbolLinks( &LOAD_INFO_REPORTER::GetInstance() ); // Update all symbol library links for all sheets. allSheets.ClearEditFlags(); // Set up the default netclass wire & bus width based on imported wires & buses. diff --git a/eeschema/sch_io/eagle/sch_io_eagle.cpp b/eeschema/sch_io/eagle/sch_io_eagle.cpp index 54a469e756..8a995588ba 100644 --- a/eeschema/sch_io/eagle/sch_io_eagle.cpp +++ b/eeschema/sch_io/eagle/sch_io_eagle.cpp @@ -476,7 +476,7 @@ SCH_SHEET* SCH_IO_EAGLE::LoadSchematicFile( const wxString& aFileName, SCHEMATIC m_pi->SaveLibrary( getLibFileName().GetFullPath() ); SCH_SCREENS allSheets( m_rootSheet ); - allSheets.UpdateSymbolLinks(); // Update all symbol library links for all sheets. + allSheets.UpdateSymbolLinks( &LOAD_INFO_REPORTER::GetInstance() ); // Update all symbol library links for all sheets. return m_rootSheet; } diff --git a/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr_parser.cpp b/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr_parser.cpp index 509004cee7..9b06ce4764 100644 --- a/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr_parser.cpp +++ b/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr_parser.cpp @@ -593,7 +593,7 @@ LIB_SYMBOL* SCH_IO_KICAD_SEXPR_PARSER::parseLibSymbol( LIB_SYMBOL_MAP& aSymbolLi } catch( const IO_ERROR& e ) { - wxLogError( e.What() ); + m_parseWarnings.push_back( e.What() ); } SyncLineReaderWith( embeddedFilesParser ); @@ -3012,7 +3012,7 @@ void SCH_IO_KICAD_SEXPR_PARSER::ParseSchematic( SCH_SHEET* aSheet, bool aIsCopya } catch( const PARSE_ERROR& e ) { - wxLogError( e.What() ); + m_parseWarnings.push_back( e.What() ); } SyncLineReaderWith( embeddedFilesParser ); diff --git a/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr_parser.h b/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr_parser.h index 173c59ae43..4e0a22a6a7 100644 --- a/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr_parser.h +++ b/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr_parser.h @@ -113,6 +113,12 @@ public: int GetParsedRequiredVersion() const { return m_requiredVersion; } + /** + * Return any non-fatal parse warnings that occurred during parsing. + * These are errors that were handled gracefully but should be reported to the user. + */ + const std::vector& GetParseWarnings() const { return m_parseWarnings; } + private: // Group membership info refers to other Uuids in the file. // We don't want to rely on group declarations being last in the file, so @@ -276,6 +282,8 @@ private: int m_maxError; std::vector m_groupInfos; + + std::vector m_parseWarnings; ///< Non-fatal warnings collected during parsing }; #endif // SCH_IO_KICAD_SEXPR_PARSER_H_ diff --git a/include/footprint_info.h b/include/footprint_info.h index 8abf5e03b5..bfad3205a9 100644 --- a/include/footprint_info.h +++ b/include/footprint_info.h @@ -226,6 +226,11 @@ public: return error; } + void PushError( std::unique_ptr aError ) + { + m_errors.move_push( std::move( aError ) ); + } + /** * Read all the footprints provided by the combination of aTable and aNickname. * @@ -243,6 +248,12 @@ public: void DisplayErrors( wxTopLevelWindow* aCaller = nullptr ); + /** + * Returns all accumulated errors as a newline-separated string for display in the + * status bar. This consumes the errors (pops them from the queue). + */ + wxString GetErrorMessages(); + FOOTPRINT_LIBRARY_ADAPTER* GetAdapter() const { return m_adapter; } /** diff --git a/include/libraries/library_manager.h b/include/libraries/library_manager.h index b0d5634547..a8015ae91d 100644 --- a/include/libraries/library_manager.h +++ b/include/libraries/library_manager.h @@ -148,6 +148,9 @@ public: /// Returns a list of all library nicknames and their status (even if they failed to load) std::vector> GetLibraryStatuses() const; + /// Returns all library load errors as newline-separated strings for display + wxString GetLibraryLoadErrors() const; + void ReloadLibraryEntry( const wxString& aNickname, LIBRARY_TABLE_SCOPE aScope = LIBRARY_TABLE_SCOPE::BOTH ); diff --git a/include/pgm_base.h b/include/pgm_base.h index ffd767698a..6092dfa653 100644 --- a/include/pgm_base.h +++ b/include/pgm_base.h @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include @@ -49,6 +50,8 @@ class wxWindow; class wxSplashScreen; class wxSingleInstanceChecker; +class KISTATUSBAR; +struct LOAD_MESSAGE; struct BACKGROUND_JOB; class BACKGROUND_JOBS_MONITOR; class NOTIFICATIONS_MANAGER; @@ -360,6 +363,28 @@ public: */ void PreloadDesignBlockLibraries( KIWAY* aKiway ); + /** + * Register a status bar to receive library load warning messages. + * Multiple status bars can be registered (one per open frame). + */ + void RegisterLibraryLoadStatusBar( KISTATUSBAR* aStatusBar ); + + /** + * Unregister a status bar from receiving library load warning messages. + */ + void UnregisterLibraryLoadStatusBar( KISTATUSBAR* aStatusBar ); + + /** + * Add library load messages to all registered status bars. + * Thread-safe - can be called from background threads. + */ + void AddLibraryLoadMessages( const std::vector& aMessages ); + + /** + * Clear library load messages from all registered status bars. + */ + void ClearLibraryLoadMessages(); + /** * wxWidgets on MSW tends to crash if you spool up more than one print job at a time. */ @@ -432,6 +457,9 @@ protected: std::future m_libraryPreloadReturn; std::atomic_bool m_libraryPreloadInProgress; std::atomic_bool m_libraryPreloadAbort; + + std::vector m_libraryLoadStatusBars; + mutable std::mutex m_libraryLoadStatusBarsMutex; }; diff --git a/include/string_utils.h b/include/string_utils.h index 3ad5f9737f..f38a3fef21 100644 --- a/include/string_utils.h +++ b/include/string_utils.h @@ -511,4 +511,17 @@ KICOMMON_API wxString GetDefaultVariantName(); KICOMMON_API int SortVariantNames( const wxString& aLhs, const wxString& aRhs ); +struct LOAD_MESSAGE; + +/** + * Parse library load error messages, extracting user-facing information while + * stripping internal code locations. + * + * @param aErrorString is the raw error string from GetLibraryLoadErrors() + * @param aSeverity is the severity to assign to all extracted messages + * @return vector of LOAD_MESSAGE with cleaned error text + */ +KICOMMON_API std::vector ExtractLibraryLoadErrors( const wxString& aErrorString, + int aSeverity ); + #endif // STRING_UTILS_H diff --git a/include/widgets/kistatusbar.h b/include/widgets/kistatusbar.h index 560d722f14..ec46c12a5d 100644 --- a/include/widgets/kistatusbar.h +++ b/include/widgets/kistatusbar.h @@ -27,6 +27,7 @@ #include #include +#include #include #include @@ -116,11 +117,23 @@ public: void SetLoadWarningMessages( const wxString& aMessages ); void ClearLoadWarningMessages(); + /** + * Add warning/error messages thread-safely. + * Can be called from any thread. UI update is deferred to main thread. + */ + void AddLoadWarningMessages( const std::vector& aMessages ); + + /** + * Get current message count (thread-safe). + */ + size_t GetLoadWarningCount() const; + private: void onSize( wxSizeEvent& aEvent ); void onBackgroundProgressClick( wxMouseEvent& aEvent ); void onNotificationsIconClick( wxCommandEvent& aEvent ); void onLoadWarningsIconClick( wxCommandEvent& aEvent ); + void updateWarningUI(); ///< Update warning button visibility and badge (main thread only) enum class FIELD { @@ -139,6 +152,7 @@ private: wxStaticText* m_backgroundTxt; BITMAP_BUTTON* m_notificationsButton; BITMAP_BUTTON* m_warningButton; + mutable std::mutex m_loadWarningMutex; ///< Protects m_loadWarningMessages std::vector m_loadWarningMessages; int m_normalFieldsCount; STYLE_FLAGS m_styleFlags; diff --git a/kicad/kicad_manager_frame.cpp b/kicad/kicad_manager_frame.cpp index 202e3cc124..781f19e405 100644 --- a/kicad/kicad_manager_frame.cpp +++ b/kicad/kicad_manager_frame.cpp @@ -172,6 +172,7 @@ KICAD_MANAGER_FRAME::KICAD_MANAGER_FRAME( wxWindow* parent, const wxString& titl CreateStatusBar( 2 ); Pgm().GetBackgroundJobMonitor().RegisterStatusBar( (KISTATUSBAR*) GetStatusBar() ); Pgm().GetNotificationsManager().RegisterStatusBar( (KISTATUSBAR*) GetStatusBar() ); + Pgm().RegisterLibraryLoadStatusBar( (KISTATUSBAR*) GetStatusBar() ); GetStatusBar()->SetFont( KIUI::GetStatusFont( this ) ); // Give an icon @@ -324,6 +325,7 @@ KICAD_MANAGER_FRAME::~KICAD_MANAGER_FRAME() Pgm().GetBackgroundJobMonitor().UnregisterStatusBar( (KISTATUSBAR*) GetStatusBar() ); Pgm().GetNotificationsManager().UnregisterStatusBar( (KISTATUSBAR*) GetStatusBar() ); + Pgm().UnregisterLibraryLoadStatusBar( (KISTATUSBAR*) GetStatusBar() ); // Shutdown all running tools if( m_toolManager ) @@ -383,7 +385,10 @@ void KICAD_MANAGER_FRAME::onNotebookPageCloseRequest( wxAuiNotebookEvent& evt ) wxStatusBar* KICAD_MANAGER_FRAME::OnCreateStatusBar( int number, long style, wxWindowID id, const wxString& name ) { - return new KISTATUSBAR( number, this, id ); + return new KISTATUSBAR( number, this, id, + static_cast( + KISTATUSBAR::NOTIFICATION_ICON | KISTATUSBAR::CANCEL_BUTTON + | KISTATUSBAR::WARNING_ICON ) ); } diff --git a/pcbnew/cross-probing.cpp b/pcbnew/cross-probing.cpp index 36ae9584fb..d818af6939 100644 --- a/pcbnew/cross-probing.cpp +++ b/pcbnew/cross-probing.cpp @@ -57,6 +57,9 @@ #include #include #include +#include +#include +#include #include /* Execute a remote command sent via a socket on port KICAD_PCB_PORT_SERVICE_NUMBER @@ -726,8 +729,21 @@ void PCB_EDIT_FRAME::KiwayMailIn( KIWAY_EXPRESS& mail ) break; case MAIL_RELOAD_LIB: + { m_designBlocksPane->RefreshLibs(); + + // Show any footprint library load errors in the status bar + if( KISTATUSBAR* statusBar = dynamic_cast( GetStatusBar() ) ) + { + FOOTPRINT_LIBRARY_ADAPTER* adapter = PROJECT_PCB::FootprintLibAdapter( &Prj() ); + wxString errors = adapter->GetLibraryLoadErrors(); + + if( !errors.IsEmpty() ) + statusBar->SetLoadWarningMessages( errors ); + } + break; + } // many many others. default: diff --git a/pcbnew/files.cpp b/pcbnew/files.cpp index 336a2a2783..1c782e633b 100644 --- a/pcbnew/files.cpp +++ b/pcbnew/files.cpp @@ -31,7 +31,6 @@ #include #include #include -#include #include #include #include @@ -623,7 +622,6 @@ bool PCB_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, in std::placeholders::_1 ) ); } - DIALOG_HTML_REPORTER errorReporter( this ); bool failedLoad = false; try @@ -660,8 +658,10 @@ bool PCB_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, in // measure the time to load a BOARD. int64_t startTime = GetRunningMicroSecs(); #endif + // Use loadReporter for import issues - they will be shown in the status bar + // warning icon instead of a modal dialog if( config()->m_System.show_import_issues ) - pi->SetReporter( errorReporter.m_Reporter ); + pi->SetReporter( &loadReporter ); else pi->SetReporter( &NULL_REPORTER::GetInstance() ); @@ -718,12 +718,6 @@ bool PCB_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, in // compiled. Raise(); - if( errorReporter.m_Reporter->HasMessage() ) - { - errorReporter.m_Reporter->Flush(); // Build HTML messages - errorReporter.ShowModal(); - } - // Skip (possibly expensive) connectivity build here; we build it below after load SetBoard( loadedBoard, false, &progressReporter ); diff --git a/pcbnew/footprint_edit_frame.cpp b/pcbnew/footprint_edit_frame.cpp index c5baf277f7..d8a65dc5fc 100644 --- a/pcbnew/footprint_edit_frame.cpp +++ b/pcbnew/footprint_edit_frame.cpp @@ -70,6 +70,7 @@ #include #include #include +#include #include #include #include @@ -1151,7 +1152,10 @@ void FOOTPRINT_EDIT_FRAME::initLibraryTree() progressReporter.Show( false ); if( GFootprintList.GetErrorCount() ) - GFootprintList.DisplayErrors( this ); + { + if( KISTATUSBAR* statusBar = dynamic_cast( GetStatusBar() ) ) + statusBar->SetLoadWarningMessages( GFootprintList.GetErrorMessages() ); + } m_adapter = FP_TREE_SYNCHRONIZING_ADAPTER::Create( this, footprints ); auto adapter = static_cast( m_adapter.get() ); diff --git a/pcbnew/footprint_info_impl.cpp b/pcbnew/footprint_info_impl.cpp index 697acf05e5..bf0538937f 100644 --- a/pcbnew/footprint_info_impl.cpp +++ b/pcbnew/footprint_info_impl.cpp @@ -45,20 +45,32 @@ void FOOTPRINT_INFO_IMPL::load() FOOTPRINT_LIBRARY_ADAPTER* adapter = m_owner->GetAdapter(); wxCHECK( adapter, /* void */ ); - std::unique_ptr footprint( adapter->LoadFootprint( m_nickname, m_fpname, false ) ); - - if( footprint == nullptr ) // Should happen only with malformed/broken libraries + try { + std::unique_ptr footprint( adapter->LoadFootprint( m_nickname, m_fpname, false ) ); + + if( footprint == nullptr ) // Should happen only with malformed/broken libraries + { + m_pad_count = 0; + m_unique_pad_count = 0; + } + else + { + m_pad_count = footprint->GetPadCount( DO_NOT_INCLUDE_NPTH ); + m_unique_pad_count = footprint->GetUniquePadCount( DO_NOT_INCLUDE_NPTH ); + m_keywords = footprint->GetKeywords(); + m_doc = footprint->GetLibDescription(); + } + } + catch( const IO_ERROR& ioe ) + { + // Store error in the owner's error list for later display + if( m_owner ) + m_owner->PushError( std::make_unique( ioe ) ); + m_pad_count = 0; m_unique_pad_count = 0; } - else - { - m_pad_count = footprint->GetPadCount( DO_NOT_INCLUDE_NPTH ); - m_unique_pad_count = footprint->GetUniquePadCount( DO_NOT_INCLUDE_NPTH ); - m_keywords = footprint->GetKeywords(); - m_doc = footprint->GetLibDescription(); - } m_loaded = true; } diff --git a/pcbnew/footprint_library_adapter.cpp b/pcbnew/footprint_library_adapter.cpp index beb3ba496d..701d088567 100644 --- a/pcbnew/footprint_library_adapter.cpp +++ b/pcbnew/footprint_library_adapter.cpp @@ -364,12 +364,20 @@ FOOTPRINT* FOOTPRINT_LIBRARY_ADAPTER::LoadFootprint( const wxString& aNickname, { if( std::optional lib = fetchIfLoaded( aNickname ) ) { - if( FOOTPRINT* footprint = pcbplugin( *lib )->FootprintLoad( getUri( ( *lib )->row ), aName ) ) + try { - LIB_ID id = footprint->GetFPID(); - id.SetLibNickname( ( *lib )->row->Nickname() ); - footprint->SetFPID( id ); - return footprint; + if( FOOTPRINT* footprint = pcbplugin( *lib )->FootprintLoad( getUri( ( *lib )->row ), aName ) ) + { + LIB_ID id = footprint->GetFPID(); + id.SetLibNickname( ( *lib )->row->Nickname() ); + footprint->SetFPID( id ); + return footprint; + } + } + catch( const IO_ERROR& ioe ) + { + wxLogTrace( traceLibraries, "LoadFootprint: error loading %s:%s: %s", + aNickname, aName, ioe.What() ); } } else diff --git a/pcbnew/footprint_viewer_frame.cpp b/pcbnew/footprint_viewer_frame.cpp index 22a6ddaceb..2f3b602c2a 100644 --- a/pcbnew/footprint_viewer_frame.cpp +++ b/pcbnew/footprint_viewer_frame.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -492,7 +493,8 @@ void FOOTPRINT_VIEWER_FRAME::ReCreateFootprintList() if( fp_info_list->GetErrorCount() ) { - fp_info_list->DisplayErrors( this ); + if( KISTATUSBAR* statusBar = dynamic_cast( GetStatusBar() ) ) + statusBar->SetLoadWarningMessages( fp_info_list->GetErrorMessages() ); // For footprint libraries that support one footprint per file, there may have been // valid footprints read so show the footprints that loaded properly. diff --git a/pcbnew/generate_footprint_info.cpp b/pcbnew/generate_footprint_info.cpp index 4797650b96..52e31c3a11 100644 --- a/pcbnew/generate_footprint_info.cpp +++ b/pcbnew/generate_footprint_info.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include @@ -126,7 +127,8 @@ public: } catch( const IO_ERROR& ioe ) { - wxLogError( _( "Error loading footprint %s from library '%s'." ) + wxS( "\n%s" ), + // Log to trace instead of error to avoid popup dialogs + wxLogTrace( traceLibraries, "Error loading footprint %s from library '%s': %s", m_lib_id.GetLibItemName().wx_str(), m_lib_id.GetLibNickname().wx_str(), ioe.What() ); diff --git a/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp b/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp index c9ef1c78ad..65f30a8773 100644 --- a/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp +++ b/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.cpp @@ -190,6 +190,17 @@ void FP_CACHE::Load() footprint->SetFPID( LIB_ID( wxEmptyString, fpName ) ); m_footprints.insert( fpName, new FP_CACHE_ENTRY( footprint, fn ) ); + + // Collect any non-fatal parse warnings + for( const wxString& warning : parser.GetParseWarnings() ) + { + if( !cacheError.IsEmpty() ) + cacheError += wxT( "\n\n" ); + + cacheError += wxString::Format( _( "Warning in file '%s'" ) + '\n', + fn.GetFullPath() ); + cacheError += warning; + } } catch( const IO_ERROR& ioe ) { diff --git a/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr_parser.cpp b/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr_parser.cpp index 479ecaa7c7..e337c2e64c 100644 --- a/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr_parser.cpp +++ b/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr_parser.cpp @@ -73,6 +73,7 @@ #include #include #include +#include // For some reason wxWidgets is built with wxUSE_BASE64 unset so expose the wxWidgets // base64 code. Needed for PCB_REFERENCE_IMAGE @@ -1173,7 +1174,7 @@ BOARD* PCB_IO_KICAD_SEXPR_PARSER::parseBOARD_unchecked() } catch( const PARSE_ERROR& e ) { - wxLogError( e.What() ); + m_parseWarnings.push_back( e.What() ); } SyncLineReaderWith( embeddedFilesParser ); @@ -5306,7 +5307,7 @@ FOOTPRINT* PCB_IO_KICAD_SEXPR_PARSER::parseFOOTPRINT_unchecked( wxArrayString* a } catch( const PARSE_ERROR& e ) { - wxLogError( e.What() ); + m_parseWarnings.push_back( e.What() ); } SyncLineReaderWith( embeddedFilesParser ); diff --git a/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr_parser.h b/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr_parser.h index 5ec40952e3..80f79794db 100644 --- a/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr_parser.h +++ b/pcbnew/pcb_io/kicad_sexpr/pcb_io_kicad_sexpr_parser.h @@ -136,6 +136,12 @@ public: */ bool IsValidBoardHeader(); + /** + * Return any non-fatal parse warnings that occurred during parsing. + * These are errors that were handled gracefully but should be reported to the user. + */ + const std::vector& GetParseWarnings() const { return m_parseWarnings; } + private: // Group membership info refers to other Uuids in the file. @@ -449,6 +455,8 @@ private: std::vector m_generatorInfos; std::function m_queryUserCallback; + + std::vector m_parseWarnings; ///< Non-fatal warnings collected during parsing }; diff --git a/pcbnew/pcbnew.cpp b/pcbnew/pcbnew.cpp index b38c593b61..288f49c953 100644 --- a/pcbnew/pcbnew.cpp +++ b/pcbnew/pcbnew.cpp @@ -62,7 +62,12 @@ #include #include #include +#include #include +#include +#include + +#include #include "invoke_pcb_dialog.h" #include @@ -611,6 +616,8 @@ void IFACE::PreloadLibraries( KIWAY* aKiway ) if( m_libraryPreloadInProgress.load() ) return; + Pgm().ClearLibraryLoadMessages(); + m_libraryPreloadBackgroundJob = Pgm().GetBackgroundJobMonitor().Create( _( "Loading Footprint Libraries" ) ); @@ -659,9 +666,47 @@ void IFACE::PreloadLibraries( KIWAY* aKiway ) adapter->BlockUntilLoaded(); + // Collect and report library load errors from adapter + wxString errors = adapter->GetLibraryLoadErrors(); + + wxLogTrace( traceLibraries, "pcbnew PreloadLibraries: adapter errors.IsEmpty()=%d, length=%zu", + errors.IsEmpty(), errors.length() ); + + if( !errors.IsEmpty() ) + { + std::vector messages = ExtractLibraryLoadErrors( errors, RPT_SEVERITY_ERROR ); + + wxLogTrace( traceLibraries, " -> adapter: collected %zu messages", messages.size() ); + + if( !messages.empty() ) + Pgm().AddLibraryLoadMessages( messages ); + } + else + { + wxLogTrace( traceLibraries, " -> no errors from footprint adapter" ); + } + // TODO: Remove once fp-info-cache isn't a thing GFootprintList.ReadFootprintFiles( adapter, nullptr, reporter.get() ); + // Also collect errors from GFootprintList + wxLogTrace( traceLibraries, " -> GFootprintList.GetErrorCount()=%u", GFootprintList.GetErrorCount() ); + + if( GFootprintList.GetErrorCount() > 0 ) + { + std::vector messages = + ExtractLibraryLoadErrors( GFootprintList.GetErrorMessages(), RPT_SEVERITY_ERROR ); + + wxLogTrace( traceLibraries, " -> GFootprintList: collected %zu messages", messages.size() ); + + if( !messages.empty() ) + Pgm().AddLibraryLoadMessages( messages ); + } + else + { + wxLogTrace( traceLibraries, " -> no errors from GFootprintList" ); + } + Pgm().GetBackgroundJobMonitor().Remove( m_libraryPreloadBackgroundJob ); m_libraryPreloadBackgroundJob.reset(); m_libraryPreloadInProgress.store( false ); diff --git a/pcbnew/widgets/panel_footprint_chooser.cpp b/pcbnew/widgets/panel_footprint_chooser.cpp index ae8d6899a4..70f09dc2e8 100644 --- a/pcbnew/widgets/panel_footprint_chooser.cpp +++ b/pcbnew/widgets/panel_footprint_chooser.cpp @@ -45,6 +45,7 @@ #include #include #include +#include // When a new footprint is selected, a custom event is sent, for instance to update // 3D viewer. So define a FP_SELECTION_EVENT event @@ -79,7 +80,11 @@ PANEL_FOOTPRINT_CHOOSER::PANEL_FOOTPRINT_CHOOSER( PCB_BASE_FRAME* aFrame, wxTopL delete progressReporter; if( GFootprintList.GetErrorCount() ) - GFootprintList.DisplayErrors( aParent ); + { + // Show errors in status bar instead of popup dialog + if( KISTATUSBAR* statusBar = dynamic_cast( aFrame->GetStatusBar() ) ) + statusBar->SetLoadWarningMessages( GFootprintList.GetErrorMessages() ); + } m_adapter = FP_TREE_MODEL_ADAPTER::Create( aFrame, footprints ); FP_TREE_MODEL_ADAPTER* adapter = static_cast( m_adapter.get() );