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

for #4154 Only enable Vext regulator when needed for either Screen or GPS #4527

Merged
merged 45 commits into from
Sep 2, 2024

Conversation

geeksville
Copy link
Member

@geeksville geeksville commented Aug 21, 2024

(just opening now for CI notifications)

notes to self:

Changes in this PR

  • change GPS code to use virtual gpio for pwr enable (so that we can manage more cleverly)
  • change TFT code to use a virtual gpio for backlight enable
  • cleanup a bunch of redundant variant defs which led to an explosion of unneeded ifdefs
  • use virt GPIOs inside screen and GPS
  • confirm that power savings works. Done: with this change when the GPS and screen are both powered down the power consumption is about 10mA lower on the USB 5V in (I haven't yet tested when running on battery, but this matches expected behavior based on the regulator datasheet)
  • confirmed that if the screen is needed the regulator is powered up as needed.

TODO still

  • test that GPS still works!

* 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
No need for _V05 and _V03 definitions - I think there was a slight misunderstanding
on how variant files are supposed to _decrease_ #ifdef code in the cpp files.
Therefore don't just randomly be writing to a GPIO numbered -1
Instead just don't try to control the backlight
NOTE: I don't have a 'wiphone' to test with, but I saw this via inspection
while cleaning up some other stuff.
@geeksville geeksville changed the title for #4154 DO NOT MERGE for #4154 Only enable Vext regulator when needed for either Screen or GPS Aug 28, 2024
thebentern and others added 16 commits August 28, 2024 11:25
* Initial support for RadioMaster Bandit.

* Different lighting can be made for Button 1 & 2 on the Bandit.
Changes to AmbientLighting will turn off af shutdown().

* Trunk

* Trunk again.
But in an attempt to avoid updating lots of files, make it default to LOW
(from src/extra_variants/README.md)

This directory tree is designed to solve two problems.

- The ESP32 arduino/platformio project doesn't support the nice "if initVariant() is found, call that after init" behavior of the nrf52 builds (they use initVariant() internally).
- Over the years a lot of 'board specific' init code has been added to init() in main.cpp. It would be great to have a general/clean mechanism to allow developers to specify board specific/unique code in a clean fashion without mucking in main.

So we are borrowing the initVariant() ideas here (by using weak gcc references). You can now define lateInitVariant() if your board needs it.

If you'd like a board specific variant to be run, add the variant.cpp file to an appropriately named
subdirectory and check for \_VARIANT_boardname in the cpp file (so that your code is only built for your board).
You'll need to define \_VARIANT_boardname in your corresponding variant.h file.
See existing boards for examples.

This approach has no added runtime cost.
…htastic#4516)

security option.  Per discussion in meshtastic#4375
no need to preserve the old options when changing to this new simpler
single boolean because they were newish, rarely used and only for 'advanced'
developers.
devcontainers can be used by IDEs/editors like VS Code to create a standardized development environment in a container
@CLAassistant
Copy link

CLAassistant commented Aug 28, 2024

CLA assistant check
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.
11 out of 12 committers have signed the CLA.

✅ geeksville
✅ thebentern
✅ gjelsoe
✅ jp-bennett
✅ ianmcorvidae
✅ jhollowe
✅ miltieIV2
✅ Nestpebble
✅ caveman99
✅ mrfyda
✅ pccr10001
❌ github-actions[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

@geeksville geeksville marked this pull request as ready for review August 29, 2024 01:12
@caveman99 caveman99 merged commit 0588d69 into meshtastic:master Sep 2, 2024
2 of 5 checks passed
@GPSFan
Copy link
Contributor

GPSFan commented Sep 2, 2024

Please make sure that there is some way to keep the GPS powered all the time. In the past setting GPS Update Interval to <=10 sec would do this. The UC6580 GNSS chip in the Wireless Tracker is very good, there are use cases where power consumption is secondary to position accuracy.

@geeksville
Copy link
Member Author

@GPSFan no worries on that aspect. This doesn't change the GPS power up/down stuff at all. If the GPS was previously enabled it will still be enabled after this change. The only change here is: when GPS is disabled (and screen is also disabled) turn off the regulator that powers them (to prevent the 10mA drain from that regulator).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.