Skip to content

Conversation

HTRamsey
Copy link
Collaborator

@HTRamsey HTRamsey commented May 13, 2025

Adds a custom filter function for the logging categories which can allow easier/finer control of the enabled categories.
Also could collect the registered categories here and remove the custom QGCLoggingCategory class.

Fixes a debug output issue too.

@dakejahl
Copy link
Contributor

On the topic of logging, can we move the log namespace as a prefix?

current

82.241 - debug: UDP State Changed: QAbstractSocket::ClosingState (qgc.comms.udplink:UDPWorker::setupSocket()::<lambda(QAbstractSocket::SocketState)>:302)

suggested

qgc.comms.udplink:: 82.241 - debug: UDP State Changed: QAbstractSocket::ClosingState (UDPWorker::setupSocket()::<lambda(QAbstractSocket::SocketState)>:302)

@HTRamsey
Copy link
Collaborator Author

HTRamsey commented May 13, 2025

Yup, it's just changing the order here if you want to make a PR
Would be nice if you could get it space aligned somehow too

@github-actions github-actions bot added the stale label Jun 13, 2025
@HTRamsey HTRamsey force-pushed the dev-loggingcategory branch 2 times, most recently from 9f0df8d to 857f350 Compare July 9, 2025 07:44
@HTRamsey HTRamsey requested a review from Copilot July 10, 2025 11:25
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a custom filter callback for QLoggingCategory to automatically register new “qgc.*” categories and streamlines video logging rules, while also updating the core QGCLogging class.

  • Introduces qgcCategoryFilter to wrap and extend the default Qt category filter
  • Replaces explicit video manager category rules with a wildcard
  • Updates QGCLogging class: adds destructor logging, refines message pattern, and renames logging category

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/Utilities/QGCLoggingCategory.cc Implements custom category filter, registers new categories, simplifies video rules
src/Utilities/QGCLogging.h Declares ~QGCLogging()
src/Utilities/QGCLogging.cc Changes category name, refines message handler, adds destructor log, updates pattern
Comments suppressed due to low confidence (3)

src/Utilities/QGCLoggingCategory.cc:19

  • The new filter logic around defaultCategoryFilter and qgcCategoryFilter isn’t covered by existing tests. Consider adding unit tests to verify both the default filter invocation and automatic category registration.
static QLoggingCategory::CategoryFilter defaultCategoryFilter = nullptr;

src/Utilities/QGCLoggingCategory.cc:19

  • [nitpick] The name defaultCategoryFilter may be ambiguous; consider renaming to something like previousCategoryFilter or originalCategoryFilter for clearer intent.
static QLoggingCategory::CategoryFilter defaultCategoryFilter = nullptr;

src/Utilities/QGCLoggingCategory.cc:21

  • The CategoryFilter callback must return a bool to indicate if logging is enabled; qgcCategoryFilter is declared void and does not return a value. Change its signature to return bool and return the combined result of defaultCategoryFilter(category) and your custom registration logic.
static void qgcCategoryFilter(QLoggingCategory *category)

if (categoryName.startsWith("qgc.")) {
if (!QGCLoggingCategoryRegister::instance()->registeredCategories().contains(categoryName)) {
QGCLoggingCategoryRegister::instance()->registerCategory(categoryName);
// category->setEnabled(QtDebugMsg, true);
Copy link

Copilot AI Jul 10, 2025

Choose a reason for hiding this comment

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

[nitpick] This commented-out call is left over in the filter callback; if it’s no longer needed, it should be removed to keep the code clean and maintainable.

Suggested change
// category->setEnabled(QtDebugMsg, true);

Copilot uses AI. Check for mistakes.

@github-actions github-actions bot removed the stale label Jul 15, 2025
@HTRamsey HTRamsey force-pushed the dev-loggingcategory branch from 857f350 to f502dd3 Compare July 20, 2025 21:52
@HTRamsey HTRamsey marked this pull request as ready for review July 20, 2025 22:35
@HTRamsey HTRamsey merged commit 9e06508 into mavlink:master Jul 20, 2025
14 checks passed
@HTRamsey HTRamsey deleted the dev-loggingcategory branch July 20, 2025 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants