PowerMon/StructuredLogging support

[geeksville]

Mostly done but I'd like to request review/merge soonish (because I've got too many open branches with changes and tire of merging ;-) ) - see bottom comment.

Fixes #603

[CLAassistant]

All committers have signed the CLA.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: geeksville
:x: github-actions

---

github-actions seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[geeksville]

@ianmcorvidae and anyone else who is appropriate: How critical is it to keep python 3.8 support? I'd like to use the pandas data processing library for the 'analysis' apis/reporting/visualization (as opposed to device APIs). But that lib requires min python of 3.9.

I've checked and even my raspberry pi could do that (raspbian is on 3.11 now).

I would kinda rather not making a new 'meshtastic-analysis' python project. instead I'd like to have some new classes/code in meshtastic.analysis of this project.

[thebentern]

Python 3.8 is 5 years old now. I think that's fair to dump it, honestly.

[ianmcorvidae]

Yeah -- as I was saying in discord the other day, 3.8 is hitting formal end of life in October anyway, so I'm happy to drop support for it as part of this. Once we do so, I'll make sure to create one last 3.8-supporting version that'll warn users about it, just in case there's folks lingering on the old version.

[geeksville]

okay coolbeans @ianmcorvidae - I'm going to make this powermon/structured logging branch assume/require 3.8 and it can go in after that release.

[codecov[bot]]

Codecov Report

:x: Patch coverage is 37.37624% with 253 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 59.09%. Comparing base (ae904f6) to head (6332798). :warning: Report is 494 commits behind head on master.

Files with missing lines	Patch %	Lines
meshtastic/slog/slog.py	35.84%	68 Missing :warning:
meshtastic/powermon/ppk2.py	20.98%	64 Missing :warning:
meshtastic/slog/arrow.py	30.23%	30 Missing :warning:
meshtastic/powermon/stress.py	35.71%	18 Missing :warning:
meshtastic/__main__.py	60.46%	17 Missing :warning:
meshtastic/powermon/riden.py	37.03%	17 Missing :warning:
meshtastic/mesh_interface.py	27.27%	16 Missing :warning:
meshtastic/stream_interface.py	20.00%	12 Missing :warning:
meshtastic/powermon/power_supply.py	63.63%	8 Missing :warning:
meshtastic/ble_interface.py	0.00%	2 Missing :warning:
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #607      +/-   ##
==========================================
- Coverage   61.63%   59.09%   -2.54%     
==========================================
  Files          14       23       +9     
  Lines        3039     3413     +374     
==========================================
+ Hits         1873     2017     +144     
- Misses       1166     1396     +230     

Flag	Coverage Δ
unittests	59.09% <37.37%> (-2.54%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ianmcorvidae]

I hear from another PR that you're out hiking so no rush in responding here :slightly_smiling_face:, but:

I was wondering if you'd be fine with me grabbing your LogRecord work from here after #627 goes in and amending both the code here and that code to send a consistent pubsub message that includes the full LogRecord so we can handle them consistently (and make the log lines available to library users, too). I don't want to introduce unnecessary conflicts for you to have to undo but given that stuff's making it into firmware it might be nice to get it in sooner/before the PowerMon stuff is finished. Thoughts?

[geeksville]

Hi hi,

Does this change look okay for ya'll to go in? I'd kinda like for this to to in now even though I still have a few more changes coming (just localized to the new PowerStress class). The runtime impact of powermon & powerstress is a small amount of CODE size and a tiny about of RAM. If the corresponding config variable is not set, the code change at runtime is minimal.

If modules are being excluded it is completely removed from the build.

I'm submitting for review now because I had to touch a fair # of files and I've got too many open WIP branches ;-).

For early docs see https://github.com/meshtastic/firmware/issues/4136. However after I'm finished with this feature (a weekish?) I'll move much of that text to become a new doc page and tutorial about how to do power measurements on boards. The main remaining work is not on the firmware or python client side but rather the analysis reporting chain (which in in a new meshtastic.analysis submodule in the python code).

[geeksville]

Hi @ianmcorvidae Do you have any advice on how to make the code coverage tool happy with this? These new classes are mostly about hardware but I'm happy to add new meshtastic/tests that use my SimPowerMeter. Are the existing tests mocked enough that I can use a MeshInterface to talk to something and get through the config message flow / reach the later PowerStress stuff?

I assume I'll need to somehow mock up the PowerStress module/agent that is running on the (sim) UUT.

Can I sign up to do this 'later'? ;-) Because I kinda dread doing a bunch of mocking on PowerStress and then having to keep updating that mocking while I'm still developing/changing the feature.

[ianmcorvidae]

I'll try to give this a review later tonight or tomorrow and see if we can just get it in. Then I can do some reorganization of log stuff anyway, make sure this and BLE are consistent and the new stuff is accessible to library users without too much pain.

As far as the code coverage, yeah, signing up to do it later is fine with me; our mocks and stuff are not in a terribly convenient place and this power stuff is still in flux even if we merge this now. #579 (if it can get done, since I haven't had much time to devote to planning it) would hopefully make testing in general easier as well, fingers crossed.

[geeksville]

Looks generally good to me. Got a few questions, particularly about potentially passing LogRecords in full to pubsub instead of just the messages, so we can be a little more consistent with that stuff.

Yeah - my plan (sound okay to you @thebentern) is that the singleton Logger instance will do the formatting at the beginning and then be dispatching LogRecords to all of the observers who want to are handing out logs to their clients (serial, TCP, BLE)

I don't know as much about the actual power monitoring stuff, but as far as I'm aware it seems fine, and I know you're planning to keep updating/changing that, so I think we can let that happen and make any other changes we need to after it settles.

yah - my hope is those changes (powermon/powerstress) will be pretty localized.

Since this will drop our 3.8 support and also since it depends on 2.3.15 protobufs, I'll make a release before merging it that's 2.3.14 and mark it as the last 3.8 release.

smart!

[geeksville]

I hear from another PR that you're out hiking so no rush in responding here 🙂, but:

I was wondering if you'd be fine with me grabbing your LogRecord work from here after #627 goes in and amending both the code here and that code to send a consistent pubsub message that includes the full LogRecord so we can handle them consistently (and make the log lines available to library users, too). I don't want to introduce unnecessary conflicts for you to have to undo but given that stuff's making it into firmware it might be nice to get it in sooner/before the PowerMon stuff is finished. Thoughts?

Oh yes if you want to do that it would be awesome! It will allow me to focus on my power goo stuff anyways!

[ianmcorvidae]

Looks generally good to me. Got a few questions, particularly about potentially passing LogRecords in full to pubsub instead of just the messages, so we can be a little more consistent with that stuff.

Yeah - my plan (sound okay to you @thebentern) is that the singleton Logger instance will do the formatting at the beginning and then be dispatching LogRecords to all of the observers who want to are handing out logs to their clients (serial, TCP, BLE)

Oh -- yeah, I only mean here on the python side of things, but from what little I understand of C++ that stuff sounds good on that end too. I just want to change pub.sendMessage("meshtastic.log.line", line=line, interface=self) to pass a dict (with message, log level, etc.) instead of a string (line). Which it sounds like we're all in agreement about, haha, just don't want to accidentally volunteer myself for any C++ changes or anything either, given I'd need to spend a few weeks learning it first to even begin :sweat_smile:

[geeksville]

ok - I think all good now (except for the codecoverage which I can ignore for now?)

[geeksville]

okay for this to go in @ianmcorvidae? I have a few more (much more localized) changes coming (finishing the power mon work items) - which I could either add to this PR or start a new one. Which do you prefer?

[ianmcorvidae]

Yeah, I'm sorry. Got slammed with a busy week and weekend and didn't get back to merging this, but I'll make a 2.3.14 release shortly and then merge this to master for 2.3.15, and update the protobufs.

[geeksville]

no rush - just checking!