[Enancement]: merge device.debug_log_enabled and bluetooth.device_logging_enabled

[geeksville]

Category

Other

Hardware

Not Applicable

Firmware Version

master

Description

Hi @thebentern and @garthvh,

What do you think about merging device.debug_log_enabled and bt.device_logging_enabled? Since they both do the same thing but for different interfaces (and probably only turned on by devs). Does the iOS client/android client have any UI for these flags currently? If not I'd propose we keep only device.debug_log_enabled.

Relevant log output

No response

[garthvh]

Both apps have it but it they don't get used much yet and we can totally coordinate on consolidation, it would be nice if we could make it easy for people to turn on one toggle and get logs on their phone, and otherwise have this stuff disabled. The always on serial console has an appreciable power hit (and is a potential security issue) for repeater / router nodes.

[thebentern]

I'm in favor of this. Like you said, they're both protobuf logging mechanisms. Perhaps we deprecate the bluetooth one and rename debug_log_enabled to logrecord_api_enabled to be more clear from a developer standpoint. debug_log_enabled seems too vague at this point

[garthvh]

If we can do the same underlying thing on android I did on iOS (push our BLE logging into apple's OSLog and then allow download of the combined app / radio log) it should really help get data from users having issues. Right now we can only really help the few users who can figure out the CLI.

[geeksville]

ok @garthvh and @andrekir. Since it sounds like usage is currently low:

I'll send in protobuf changes to remove bluetooth.device_logging_enabled from the protobufs and rename device.debug_log_enabled as device.logrecord_api_enabled. The device code will only use that new name and any old clients happen to use the old boolean it will be no effect on the device.

@garthvh can I also remove the deprecated pre logrecord BLE debugging endpoints at the same time?

cool?

[andrekir]

sounds good to me.

do we still need a separate BLE characteristic for logs now that it's using the LogRecord fromRadio?

[garthvh]

sounds good to me.

do we still need a separate BLE characteristic for logs now that it's using the LogRecord fromRadio?

That is a good question, @thebentern?

[garthvh]

ok @garthvh and @andrekir. Since it sounds like usage is currently low:

I'll send in protobuf changes to remove bluetooth.device_logging_enabled from the protobufs and rename device.debug_log_enabled as device.logrecord_api_enabled. The device code will only use that new name and any old clients happen to use the old boolean it will be no effect on the device.

@garthvh can I also remove the deprecated pre logrecord BLE debugging endpoints at the same time?

cool?

Yeah I think ben left it in place but I will consolidate everything, feel free to remove old stuff.

[thebentern]

Since it was only shipped in one version in the past behind our beta builds, I think we can safely remove it.

[geeksville]

ok starting on this now.