Skip to content
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

Heltec Tracker v1.1 U6 wastes power #4154

Open
geeksville opened this issue Jun 21, 2024 · 23 comments
Open

Heltec Tracker v1.1 U6 wastes power #4154

geeksville opened this issue Jun 21, 2024 · 23 comments
Assignees
Labels
enhancement New feature or request pinned Exclude from stale processing

Comments

@geeksville
Copy link
Member

per discussion with @HarukiToreda and @todd-herbert

https://github.com/HarukiToreda/Meshtastic-Experiments?tab=readme-ov-file#power-measured-on-meshtastic-firmware-2310-from-battery

was taking some measurements not too long ago, and it does look like it's consistently drawing 50mA more than similar boards

ooh - just from looking at the schematic I think I see a problem: GPIO3 on the CPU is used to control the enable for the TFT screen power supply (U6). I don't see anything variant.h for that board that is setting that up. So we might always be leaving that (expensive) supply on. I'll check tomorrowish and if so, have it only on when we want the display powered.
oh i see that supply is shared with gps. But when GPS and screen are both not needed we can turn it off.

@geeksville geeksville self-assigned this Jun 21, 2024
@geeksville geeksville converted this from a draft issue Jun 21, 2024
@geeksville
Copy link
Member Author

geeksville commented Jun 21, 2024

If this is indeed the problem:

I think I'm going to add the concept of a SharedGpio and Gpio micro helper classes. Then have both the GPS enable code and the screen enable code use an instance of this little class to turn their power/on off.

A SharedGpio(Gpio *inA, Gpio *inB, Gpio *out) will be a subclass of Gpio that turns on out if inA OR inB is set to on (initially - later if useful we could add an opcode like AND, XOR, whatever is needed)

@todd-herbert and @HarukiToreda thoughts on this idea?

@HarukiToreda
Copy link
Contributor

HarukiToreda commented Jun 21, 2024

intruducing SharedGpio is good concept to have. The Heltec Wireless paper for example despite having a similar board and a very efficient screen, draws as much current as the Hetec v3 with a screen on, something is guzzling lots of power, I suspect something similar may be occurring, this will help as an example to tackle such cases in the future.

@GUVWAF
Copy link
Member

GUVWAF commented Jun 21, 2024

Related: #3760.

@lyusupov
Copy link

lyusupov commented Jun 21, 2024

image

image

@todd-herbert
Copy link
Contributor

todd-herbert commented Jun 22, 2024

The Heltec Wireless paper for example despite having a similar board and a very efficient screen, draws as much current as the Hetec v3 with a screen on, something is guzzling lots of power

Now that's something I can probably play with, at least to see if disabling any particular part of the code changes the behavior. For what it's worth, it might be worth doing some more measurement with the Wireless Paper. I've definitely seen that high-idle current, but I have a funny feeling I've also a much lower idle current at times.

I see that you are only measuring 7mA difference with screen-on vs screen-off for Heltec V3. Maybe the screen is a bit of a red herring here?

Anecdotally, one of the the local guys in my area claims big Meshtastic power-savings by under-clocking the ESP32-S3 processor to 20MHz, but no idea just how badly this could break everything?

Update: with an example sketch (non-Meshtastic), I dropped the processor speed from 240MHz to 20MHz, and the power consumption fell from ~65mA to ~10mA

@todd-herbert
Copy link
Contributor

todd-herbert commented Jun 22, 2024

If this is indeed the problem:

I think I'm going to add the concept of a SharedGpio and Gpio micro helper classes. Then have both the GPS enable code and the screen enable code use an instance of this little class to turn their power/on off.

A SharedGpio(Gpio *inA, Gpio *inB, Gpio *out) will be a subclass of Gpio that turns on out if inA OR inB is set to on (initially - later if useful we could add an opcode like AND, XOR, whatever is needed)

@todd-herbert and @HarukiToreda thoughts on this idea?

It seems totally sensible! It'd be interesting to see how much power might be saved there.


Actually, one thing that's just come to mind: I spotted the other week is that UC6580 (the GPS on the Wireless Tracker, right?) isn't put into a standby state: GPS.cpp L970. I'm not sure what the technical reason for that is though; that's well over my head! Does the power consumption drop if the GPS is disabled? How about if position.gps_update_interval is set longer than 20 minutes? Could be a good clue.

I'm having a look at detangling that class slightly, after having tangled it slightly more myself just recently.. oops. Not sure what standby states are available with the UC6580. My first instinct is to bother @jp-bennett and @GPSFan for hardware info! Edit: it's not super relevant to this whole discussion, but I've pushed to #4161, just to show a rough draft. Not super attached to any of it though

@jp-bennett
Copy link
Collaborator

I spotted the other week is that UC6580 (the GPS on the Wireless Tracker, right?) isn't put into a standby state: GPS.cpp L970.

That true/false determine whether to fully power off the GPS, or just drop it into standby. I think that #ifdef is from when there was only the one device using that GPS chip, and the power rail was connected to more than the GPS. Hazy memory there.

I like the idea of the GPIO class, but there's a lot of inherent complexity in the GPS power toggle.

@todd-herbert
Copy link
Contributor

todd-herbert commented Jun 22, 2024

I think that #ifdef is from when there was only the one device using that GPS chip, and the power rail was connected to more than the GPS. Hazy memory there.

All good! Had just wondered if there was some specific piece of technical knowledge there that for me to uncover ("yeah we don't power that one down because of bug x")

@madeofstown
Copy link
Contributor

Update: with an example sketch (non-Meshtastic), I dropped the processor speed from 240MHz to 20MHz, and the power consumption fell from ~65mA to ~10mA

If the default firmware could run at 65MHz (same as rak4631) that would be amazing! The power savings would be greatly appreciated.

@todd-herbert
Copy link
Contributor

Update: with an example sketch (non-Meshtastic), I dropped the processor speed from 240MHz to 20MHz, and the power consumption fell from ~65mA to ~10mA

If the default firmware could run at 65MHz (same as rak4631) that would be amazing! The power savings would be greatly appreciated.

Just at a glance, it sounds like BLE and WiFi need that 80MHz clock on ESP32, but maybe there's some hope for this with some sort of alternative distribution channel? Don't think I should disrail this issue thread even more (oops); there's a moonshot idea about that option on the Discord server though if you're interested.

@todd-herbert
Copy link
Contributor

Just while I'm looking at #4161, I think that shared gpio class idea would probably drop nicely into GPS::writePinEN.

@StevenCellist
Copy link

Am by no means a Meshtastic expert - heck, not even a user - so this might be completely out of place. But I am a Tracker power-user. What might be good to know, is that the ADCEnable / VBat-enable should not be configured to be output-high as it drains about 60mA. Instead, input-pullup is the way to go with almost immeasurable power drain. So that may be a useful piece of information in this thread...

@todd-herbert
Copy link
Contributor

What might be good to know, is that the ADCEnable / VBat-enable should not be configured to be output-high as it drains about 60mA.

Wow yeah that's interesting! For some reason they've gone with a BJT there with no base resistor??
It looks like ADC_CTRL is only output-high for a few ms at a time, but that's no excuse for near-shorting the GPIO like this!

image

@StevenCellist
Copy link

Yeah I saw that it is only used shortly, but then again I'm no Meshtastic expert so wasn't sure if that was it or if it's used in other hidden places. So thought I'd mention it in case

@todd-herbert
Copy link
Contributor

Hey even if it's only for a brief moment, that's got to be bad for the hardware, if nothing else!

@StevenCellist
Copy link

See my earlier findings here - sorry, misreported 40mA for 60mA but still the same problem.

@GPSFan
Copy link
Contributor

GPSFan commented Jun 28, 2024

Maybe someone should ping on Aaron-Lee about why they used a BJT without base resistor. There are some specialized BJTs that have built in series base and base-to-emitter resistors, maybe Heltec was going to use one of those and the Tracker got cost reduced to use a cheaper part by some bean counter. They use Mosfets for essentially the same task in other places on the Tracker as well as BJTs with series base resistors.
They could easily swap out that part in a 1.2 version, as well as putting back control of VIO for the GPS.

@madeofstown
Copy link
Contributor

I've noticed that both of my Heltec Trackers tend to run pretty warm(specifically around the USB port)... Could it be because of the current implementation for battery monitoring?

@todd-herbert
Copy link
Contributor

todd-herbert commented Jun 29, 2024

@madeofstown I don't think it'll be that battery monitoring thing, but you never know. The excess current draw for the boards does seem to be roughly what you'd expect the GPS to consume, so that SharedGPIO class that was mentioned above might at least cut the power consumption back in line with other ESP32-S3 boards by allowing the GPS hardware to be powered down (depending on config).

@geeksville geeksville moved this from In Progress to Todo in Firmware power improvements Jun 30, 2024
geeksville added a commit to geeksville/Meshtastic-esp32 that referenced this issue Jun 30, 2024
…s...

Which is currently only tested with the LED but eventually
will be used for shared GPIO/screen power rail enable
and LED forcing (which is a sanity check in the power stress
testing)
geeksville added a commit to geeksville/Meshtastic-esp32 that referenced this issue Jun 30, 2024
…s...

Which is currently only tested with the LED but eventually
will be used for shared GPIO/screen power rail enable
and LED forcing (which is a sanity check in the power stress
testing)
@geeksville geeksville moved this from Todo to In Progress in Firmware power improvements Aug 3, 2024
thebentern added a commit that referenced this issue Aug 6, 2024
* Turn off vscode cmake prompt - we don't use cmake on meshtastic

* Add rak4631_dap variant for debugging with NanoDAP debug probe device.

* The rak device can also run freertos (which is underneath nrf52 arduino)

* Add semihosting support for nrf52840 devices
Initial platformio.ini file only supports rak4630
Default to non TCP for the semihosting log output for now...
Fixes #4135

* powermon WIP (for #4136 )

* oops - mean't to mark the _dbg variant as an 'extra' board.

* powermon wip

* Make serial port on wio-sdk-wm1110 board work
By disabling the (inaccessible) adafruit USB

* Instrument (radiolib only for now) lora for powermon
per #4136

* powermon gps support
#4136

* Add CPU deep and light sleep powermon states
#4136

* Change the board/swversion bootstring so it is a new "structured" log msg.

* powermon wip

* add example script for getting esp S3 debugging working
Not yet used but I didn't want these nasty tricks to get lost yet.

* Add PowerMon reporting for screen and bluetooth pwr.

* make power.powermon_enables config setting work.

* update to latest protobufs

* fix bogus shellcheck warning

* make powermon optional (but default enabled because tiny and no runtime impact)

* tell vscode, if formatting, use whatever our trunk formatter wants
without this flag if the user has set some other formatter (clang)
in their user level settings, it will be looking in the wrong directory
for the clang options (we want the options in .trunk/clang)

Note: formatOnSave is true in master, which means a bunch of our older
files are non compliant and if you edit them it will generate lots of
formatting related diffs.  I guess I'll start letting that happen with
my future commits ;-).

* add PowerStress module

* nrf52 arduino is built upon freertos, so let platformio debug it

* don't accidentally try to Segger ICE if we are using another ICE

* clean up RedirectablePrint::log so it doesn't have three very different implementations inline.

* remove NoopPrint - it is no longer needed

* when talking to API clients via serial, don't turn off log msgs instead encapsuate them

* fix the build - would loop forever if there were no files to send

* don't use Segger code if not talking to a Segger debugger

* when encapsulating logs, make sure the strings always has nul terminators

* nrf52 soft device will watchdog if you use ICE while BT on...
so have debugger disable bluetooth.

* Important to not print debug messages while writing to the toPhone scratch buffer

* don't include newlines if encapsulating log records as protobufs

* update to latest protobufs (needed for powermon goo)

* PowerStress WIP

* for #4154 and #4136 add concept of dependent gpios...
Which is currently only tested with the LED but eventually
will be used for shared GPIO/screen power rail enable
and LED forcing (which is a sanity check in the power stress
testing)

* fix linter warning

* Transformer is a better name for the LED input > operation > output classes

* PMW led changes to work on esp32-s3

* power stress improvements

* allow ble logrecords to be fetched either by NOTIFY or INDICATE ble types

This allows 'lossless' log reading.  If client has requested INDICATE
(rather than NOTIFY) each log record emitted via log() will have to fetched
by the client device before the meshtastic node can continue.

* Fix serious problem with nrf52 BLE logging.
When doing notifies of LogRecords it is important to use the
binary write routines - writing using the 'string' write won't work.
Because protobufs can contain \0 nuls inside of them which if being
parsed as a string will cause only a portion of the protobuf to be sent.
I noticed this because some log messages were not getting through.

* fix gpio transformer stuff to work correctly with LED_INVERTED

Thanks @todd-herbert for noticing this and the great stack trace.
The root cause was that I had accidentially shadowed outPin in a subclass
with an unneeded override.  It would break on any board that had inverted
LED power.

fixes
#4230 (review)

* Support driving multiple output gpios from one input.

While investigating #4230 (review)
I noticed in variant.h that there are now apparently newer TBEAMs than mine
that have _both_ a GPIO based power LED and the PMU based LED.  Add a splitter
so that we can drive two output GPIOs from one logical signal.

---------

Co-authored-by: Ben Meadors <[email protected]>
@geeksville
Copy link
Member Author

working on this issue now.

geeksville added a commit to geeksville/Meshtastic-esp32 that referenced this issue Aug 21, 2024
* Currently only on heltec tracker, but could use ADC_USE_PULLUP on other boards that could benefit
* Thanks @todd-herbert and @StevenCellist for the instructions ;-)
* Remove nasty Heltec_wireless #ifdefs that got somehow added to Power.cpp, instead use proper variant defs
* Cleanup adc enable/disable code a bit for less copy-paste cruft
geeksville added a commit to geeksville/Meshtastic-esp32 that referenced this issue Aug 22, 2024
geeksville added a commit to geeksville/Meshtastic-esp32 that referenced this issue Aug 22, 2024
geeksville added a commit to geeksville/Meshtastic-esp32 that referenced this issue Aug 28, 2024
geeksville added a commit to geeksville/Meshtastic-esp32 that referenced this issue Aug 28, 2024
@geeksville
Copy link
Member Author

ok the PR to make Vext management smarter (so we can save the >15mAish cost of leaving that regulator on unnecessarily) is ready. I tested it with my USB power analyzer (which unfortuately only sees the 5V USB power signal). It would be great if someone could retest when running when powered from the battery input (my uamp meter is in storage until October)

@geeksville geeksville removed their assignment Aug 29, 2024
@HarukiToreda
Copy link
Contributor

HarukiToreda commented Aug 29, 2024

ok the PR to make Vext management smarter (so we can save the >15mAish cost of leaving that regulator on unnecessarily) is ready. I tested it with my USB power analyzer (which unfortuately only sees the 5V USB power signal). It would be great if someone could retest when running when powered from the battery input (my uamp meter is in storage until October)

do you mean this PR? #4527
I tried it, i measure 164mA when screen and gps is on, same as before. I don't see a difference unfortunately. This was on a Heltec Tracker v1.1.
Before the PR when the screen turns off it runs at 145mA
After the PR when the screen is off it runs at 155mA which is higher than before
20240828_231526
20240828_220736

caveman99 added a commit that referenced this issue Sep 2, 2024
for #4154 Only enable Vext regulator when needed for either Screen or GPS
@geeksville geeksville self-assigned this Sep 2, 2024
@geeksville
Copy link
Member Author

@HarukiToreda ooh! that is interesting and differs from my testing. Alas - I'm leaving tomorrow on a long bike trip (with no computer/minimum cell-phone access). I won't be able to investigate until I return to my desk (on Oct 1, but realistically need a few days to complete a move - so should be back at meshtastic about Oct 7)

@github-actions github-actions bot added the Stale label Nov 4, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 12, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Firmware power improvements Nov 12, 2024
@caveman99 caveman99 reopened this Nov 12, 2024
@caveman99 caveman99 added enhancement New feature or request pinned Exclude from stale processing and removed Stale labels Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pinned Exclude from stale processing
Projects
Status: Done
Development

No branches or pull requests

10 participants