Add hidden 'debug-detail' preference, to allow disabling packet dumps #1357
Conversation
src/service/device.js
Outdated
| debug(packet, this.name); | ||
| if (globalThis.debugDetail) | ||
| debug(packet, this.name); | ||
| else | ||
| debug(packet.type, this.name); |
There was a problem hiding this comment.
I think this check would be better only done once (ie. swapping the callback when the setting changes), but this is good enough for now. It can always be changed later.
There was a problem hiding this comment.
Hmm, that could be tricky because only those two debug calls should be abbreviated. I suppose I could add a separate debugging call that only those two functions use, which would either be the same callback as debug() (when details are enabled), or an abbreviating version (when they aren't). That makes sense, and it's easy enough. (Then I won't need to expose debugDetail via globalThis, either.)
There was a problem hiding this comment.
@andyholmes Sorted, hopefully, though the changes are significant enough that I wouldn't say no to a second look.
There was a problem hiding this comment.
@andyholmes Whoops, hopefully you hadn't looked (again) yet, because the code wasn't done. (Forgot to save in VSCode. Argh.) Now it is.
|
Seems like a useful option. Go ahead and merge if you're ready, although since GSettings are tricky to change later if there's anything else we might want we could make this a |
Hmm. I _had_thought about defining debug contexts that could be activated/deactivated separately, so that (for instance) you could enable debugging just for one particular plugin or service. But,
|
|
If we want more flags, we can add more keys. The GNOME calculator app does actually make it super easy to get an integer by assigning bits one by one, but still not maybe a most user friendly option and definitely not something we want to find ourselves guiding users to do. |
|
This settings (which defaults to 'true') controls whether packet dumps are included in the debug output from _readLoop() and sendPacket(). If it is set to 'false', only 'packet.type' will be logged for each packet when debugging, not the entire contents.
|
LGTM, go ahead and merge when you're ready. |
|
@ferdnyc was this one ready to merge (after rebasing)? |
andyholmes
added
needs info
github-actions
bot
added
the
conflicts
|
This pull request has conflicts, please resolve those so that the changes can be evaluated. |
When attempting to debug GSConnect operations, I often find my journal overwhelmed by the massive contents of the packets being sent back and forth between the local system and my device, particularly the
kdeconnect.contacts.request_vcards_by_uid,kdeconnect.sms.messages, andkdeconnect.contacts.response_uids_timestampsmessages. When testing with a "real-world" device containing hundreds of contacts and an SMS history many hundreds of messages deep, the formatted dumps of the packet contents can be thousands of lines. And most of the time, the actual contents of the packet is of no interest to me.So, this PR adds a hidden setting 'debug-detail' (only accessible via
dconf-editoror thegsettings/dconfcommand-line tools) which controls whether packet dumps are included in the debug output from_readLoop()andsendPacket(). It defaults totrue(the current behavior), but when set tofalseonly thepacket.typewill be logged for each packet, not the entire contents.Result? Journal messages that look like this:
Which can be a hell of a lot easier to follow, at times when you don't need the packet contents.