-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Utilities: Add Logging Filter #12836
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
Conversation
On the topic of logging, can we move the log namespace as a prefix? current
suggested
|
Yup, it's just changing the order here if you want to make a PR |
9f0df8d
to
857f350
Compare
There was a problem hiding this 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
andqgcCategoryFilter
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 likepreviousCategoryFilter
ororiginalCategoryFilter
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 declaredvoid
and does not return a value. Change its signature to returnbool
and return the combined result ofdefaultCategoryFilter(category)
and your custom registration logic.
static void qgcCategoryFilter(QLoggingCategory *category)
src/Utilities/QGCLoggingCategory.cc
Outdated
if (categoryName.startsWith("qgc.")) { | ||
if (!QGCLoggingCategoryRegister::instance()->registeredCategories().contains(categoryName)) { | ||
QGCLoggingCategoryRegister::instance()->registerCategory(categoryName); | ||
// category->setEnabled(QtDebugMsg, true); |
Copilot
AI
Jul 10, 2025
There was a problem hiding this comment.
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.
// category->setEnabled(QtDebugMsg, true); |
Copilot uses AI. Check for mistakes.
857f350
to
f502dd3
Compare
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.