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

[beken-72xx] Implement deep sleep #140

Merged
merged 15 commits into from
Jul 13, 2023
Merged

Conversation

Xmister
Copy link
Contributor

@Xmister Xmister commented Jun 15, 2023

This is an initial try on Deep Sleep for the Beken platform.
Most of the code are tailored from OpenBeken.
ESPHome Changes
BDK Changes

@kuba2k2
Copy link
Member

kuba2k2 commented Jun 16, 2023

  1. Why exactly were BDK changes needed? The framework we're using is the unmodified, upstream version - it used to be built automatically, and we generally don't want to introduce any changes there.
    • if some part of the code doesn't work well, I replace it with so called "fixups", which are either headers (that are prepended to the include path), code units (that are compiled instead of the SDK unit) or single functions (that are wrapped during linking).
  2. Have you tried searching for that bk_enter_deep_sleep_mode() in BDK? Maybe it's behind some #ifdef clauses, or maybe it's static?
  3. I see that you used the "data pattern" of the library, which is usually good for complicated libs like WiFi or Serial (the latter is rewritten to data pattern, on a different branch however). What I'm not sure about is if it's absolutely needed in this case. It seems that calling enableGpioWakeup() will configure the deep sleep, allowing to start it with enter(), but enableTimerWakeup() will configure and enter deep sleep immediately. Is that intentional?
  4. Since LibreTiny v1.0.0, I'm trying to make the lower-level functions accessible to C code as well (thus, non-Arduino users, or people preferring to write in C). For that, I've introduced the LibreTiny C API, which has a unified style of functions and their names (like lt_<subsystem>_<function>()). Deep sleep could be written that way, and the DeepSleep library should be a thin wrapper over the C API - see the LibreTinyWDT class for an example.
  5. Implementing step 4 should make it unnecessary to use the data pattern - instead, you can simply store the config values (like the GPIO number and level, or timer sleep interval) as static variables inside lt_api.c unit.
  6. I see that you're building an instance of the DeepSleep class in ESPHome. As with Serial or LibreTiny classes, the instance should be global. To do that, you need to just declare it in the .cpp file of the library - the WDT example above should be good to follow.
  7. The compat/DeepSleep.h header should be unnecessary, as library directories are added to the include path anyway.

@kuba2k2 kuba2k2 added the enhancement New feature or request label Jun 16, 2023
@kuba2k2 kuba2k2 linked an issue Jun 16, 2023 that may be closed by this pull request
@Xmister
Copy link
Contributor Author

Xmister commented Jun 16, 2023

  1. The BDK was inconsistent with attribute names, eg. some part used deep_wkway, while the structure defines wake_up_way
  2. Yes, it's in manual_ps.c. It's behind #ifdef PS_SUPPORT_MANUAL_SLEEP and #ifdef CFG_USE_DEEP_PS, both seems to be set to 1 in configs now.
  3. I will try to work on the pattern differences. I wanted to progess by creating a PoC firts, then cleaning up the code, but I'm stuck with this linking error for now.

@kuba2k2
Copy link
Member

kuba2k2 commented Jun 16, 2023

Ah, I see. You're including manual_ps.h and manual_ps_pub.h in a C++ file, which links them as C++ functions. The BDK is compiled as C functions though, and that makes them look for two different symbols.
Move the two includes to sdk_private.h of the Beken core. There's a whole list of various BDK includes used throughout LT.

@Xmister
Copy link
Contributor Author

Xmister commented Jun 16, 2023

Thanks, I have sleep & gpio wakeup working now. I'll try to fix sleep duration and address your other comments and will update the PR then.

@kuba2k2
Copy link
Member

kuba2k2 commented Jun 17, 2023

Thanks. Code quality is one of the top goals of LibreTiny, that's why I care about it that much.

About the function names, I think something like lt_sleep_config_gpio() and lt_sleep_enter() would be okay. In ESPHome, we don't even have to use library wrappers, these functions can be called directly instead.

@Xmister
Copy link
Contributor Author

Xmister commented Jun 17, 2023

Hmm, okay. I thought the way WiFi is handled (having it's own class) is the preferred way instead of global functions, but I can definitely change it to functions only.
BTW sleep duration also works now, the code cleanup is the only thing remaining.

@kuba2k2
Copy link
Member

kuba2k2 commented Jun 17, 2023

The WiFi class was too complicated to write as global functions, and it was written as a class library instead to keep full compatibility with existing ESP32 libraries. As there are no libraries for Deep Sleep on ESP32 (that I was able to find), using globals should allow for more flexibility and portability of that.
Same goes for watchdog (which I think is enabled using SDK functions on ESP32) and OTA (which only has LT utilities in there).
This also allows to call the global functions inside multiple library wrappers, like in LT class and ESP class (they wrap the same low-level C functions).

@Xmister
Copy link
Contributor Author

Xmister commented Jun 17, 2023

Updated the PRs.
The only issue remaining (other than the Lint checks) is that sleep & gpio wakeup doesn't seem to work together. Separately they work fine, but if enabled together the chip wakes up instantly.
It might be a hardware limitation though.

@kuba2k2
Copy link
Member

kuba2k2 commented Jun 17, 2023

From what I can see, in the BDK, the PS_SUPPORT_MANUAL_SLEEP corresponds to "idle sleep", not to "deep sleep", so it shouldn't be necessary here. The sctrl_enter_rtos_deep_sleep function, that is called by bk_enter_deep_sleep_mode, seems to understand GPIO and RTC wakeup simultaneously, so it may be a hardware limitation (or another SDK problem - it's written poorly).

@dskw
Copy link

dskw commented Jun 17, 2023

Not sure if it's of much help but I can at least verify that BK7231N worked for me with OpenBeken with deepsleep together with GPIO wakeup. I had it running on a battery powered door sensor here

@Xmister
Copy link
Contributor Author

Xmister commented Jun 17, 2023

@kuba2k2 Right, removed the PS_SUPPORT_MANUAL_SLEEP override. This sdk is a bit weird indeed. It looks like it's some combination of the 7231N and 7231T sdk that's separate for OpenBeken. I think the attribute name differences were due to this combination as well.
I can only test with 7231N, so I don't know how will it behave on 7231T with this sdk. That's why I removed the special handling for that chip, as it was kind of written for the different 7231T sdk.

@dskw Were you using gpio and timer wakeup together? If so, are you sure it was actually sleeping for the desired time instead of waking right up after sleep?

@dskw
Copy link

dskw commented Jun 17, 2023

I did not specify a deepsleep time myself. Can't say if OpenBeken sets something internally. The only time I had set was how long the device has until it goes into deepsleep. The wakeup is then triggered by the reed sensor and wakes up the device. At least by the logs it was off for sure

@Xmister
Copy link
Contributor Author

Xmister commented Jun 17, 2023

@dskw That works the same way with these changes. The only thing not working if you set up a gpio wakeup together with a timer wakeup. i.e. Wake up 30 seconds from now OR when D1 pin goes high.

@dskw
Copy link

dskw commented Jun 17, 2023

Ahh then my mistake! Yeah didn't try that with OpenBeken. Only used the GPIO wakeup without timer indeed

@kuba2k2
Copy link
Member

kuba2k2 commented Jun 21, 2023

Hi, how is this going? Is it ready for me to review/merge?
I've seen someone saying that Tuya devices go to sleep for a specified time, and they also respond to GPIO wakeup. So that should be possible to do in some way...

Also, it might be more consistent to call the GPIO sleep function with a requested GPIO number, instead of a bit mask. That way, one would call it several times to attach the wakeup to several pins.

@Xmister
Copy link
Contributor Author

Xmister commented Jun 21, 2023

It might work together on different chipset or different SDK. I can't see any other way I could do it with this SDK, so I don't think we can make it work.

As for pin numbers I tried to have a similar interface to what ESP devices have. I can change it, but I thought we want to have similar interfaces where possible.

@kuba2k2
Copy link
Member

kuba2k2 commented Jun 22, 2023

Okay, that will do. Two last things:

  • can you make the sleep duration parameter in milliseconds? Seconds aren't really used as a unit in Arduino, most stuff is in millis.
  • BDK changes should be easy to avoid right now, as they're only in the "idle sleep" functions. The bk_misc_update_set_type can be added in LT code instead

@Xmister
Copy link
Contributor Author

Xmister commented Jun 22, 2023

@kuba2k2

  • Yeah, I was surprised as well that the BDK expects it in seconds. My idea of keeping it at seconds was to prevent confusion when someone tries to use the framework and wonders why their precise millis don't work as they expect.

  • I'll see what I can do with the BDK.

@kuba2k2
Copy link
Member

kuba2k2 commented Jun 22, 2023

Yeah, I was surprised as well that the BDK expects it in seconds

Actually, that might be not the whole truth - take a look at the bk_enter_deep_sleep_mode function:

		if ( deep_param->sleep_time > 0x1ffff ) {
			deep_param->sleep_time = 0x1ffff;
		}

		deep_param->sleep_time = 32768 * deep_param->sleep_time;

It looks like the chip measures the time in 32KHz clock ticks (which means that 2 seconds is 32768*2 = 65536 ticks).

If we skip the bk_enter_deep_sleep_mode and call sddev_control directly (or: even better, call sctrl_enter_rtos_deep_sleep directly. It's called by the whole weird sddev_control thing), we can control the sleep time more precisely (1 millisecond would be around 32 ticks then). The wrapper functions doesn't do anything useful apart from printing sleep info and disabling interrupts.

@kuba2k2
Copy link
Member

kuba2k2 commented Jun 24, 2023

I'm afraid adding multiple pins this way won't work right. If you first configure some pins, and then configure others, the edge map value will be wrong (because of &= ~). It will clear all bits configured before.

If we've already decided for using the index map, let's keep it simple. I don't know if calling that multiple times on ESP "adds" pins to the map, or replaces them. Here, ESP32 compatibility was primarily meant for the Arduino core only - things like WiFi library or the "ESP" class, or entire Arduino Wiring functions. As ESP32 doesn't have Arduino libs for deep sleep, it doesn't have to be compatible. We can even add separate methods, like lt_deep_sleep_config_gpio as well as lt_deep_sleep_add_gpio and lt_deep_sleep_remove_gpio, or something. (the latter two would get a gpio number, instead of index map)
But again, that's optional, and for me it was fine the way you did it before.

@Xmister
Copy link
Contributor Author

Xmister commented Jun 24, 2023

I don't see how it would clear all bits. Let's say we have

deep_sleep_param.gpio_index_map = 01000000
deep_sleep_param.gpio_edge_map = 01000000

already set from past. That means pin 1 will wake on edge fall.
We want pin 2 to wake from edge rise.
gpio_index_map parameter will be 00100000

deep_sleep_param.gpio_index_map = 01000000 | 00100000 = 01100000
deep_sleep_param.gpio_edge_map = 01000000 & 11011111 = 01000000

So it will be fine.
This will only zero out a bit if it was previously set to edge fall, and now we want to set to edge rise.
Note: 0 means rising edge in edge map, 1 means falling edge.

@kuba2k2
Copy link
Member

kuba2k2 commented Jun 24, 2023

Right... sorry for that, ignore me 😄 I somehow thought that gpio_index_map depends on the on_high state, idk

In this case I'd still consider adding a way to remove previously set GPIOs, as I don't think it's possible now. Not that it's very much needed.. but just for the sake of completeness.

@Xmister
Copy link
Contributor Author

Xmister commented Jun 24, 2023

@kuba2k2 I think this is pretty much ready for review.

Note: I don't really now why, but GPIO and timer wakeup works combined as well now.

@kuba2k2
Copy link
Member

kuba2k2 commented Jun 24, 2023

That's great! I can see that BDK changes weren't necessary in the end, which is also good.

There has been another (yes, another) little refactor on feature/ambz2 branch - the LT API implementation files are now split by their function (cpu, mem, ota, etc). Do you prefer this merged there already, or do you need it in the master branch ASAP?

@Xmister
Copy link
Contributor Author

Xmister commented Jun 24, 2023

I'm using it as an external_component now, so I'm not in a hurry.

external_components:
  - source: github://Xmister/libretuya-esphome@deep-sleep
    components: [ deep_sleep ]

And libretiny cache is checked out from my branch as well.

@samhunt
Copy link

samhunt commented Jul 13, 2023

@Xmister , how do you keep your version of libretiny cached? Every time I update my device (with your deep-sleep fork checked out) esphome will update with the latest kuba2k2 commit

Platform Manager: Installing git+https://github.com/kuba2k2/libretiny.git
INFO Installing git+https://github.com/kuba2k2/libretiny.git
git version 2.30.2
Cloning into '/cache/platformio/cache/tmp/pkg-installing-zahc_lk3'...
Platform Manager: [email protected]+sha.6169f68 has been installed!
INFO [email protected]+sha.6169f68 has been installed!

Alternatively, any ETA on when this might be merged into the base libretiny?

@kuba2k2
Copy link
Member

kuba2k2 commented Jul 13, 2023

If you use version: latest in the yaml, it will keep whatever version you have installed.

@kuba2k2 kuba2k2 changed the title Deep Sleep WIP [beken-72xx] Implement deep sleep Jul 13, 2023
@kuba2k2 kuba2k2 merged commit 93e0a5d into libretiny-eu:master Jul 13, 2023
2 checks passed
@daniel-dona
Copy link

Hi, do you have any minimal yaml file working?

Tried as external component and got this error:

#external_components:
#  - source: github://Xmister/libretuya-esphome@deep-sleep
#    components: [ deep_sleep ]

#deep_sleep:
#  run_duration: 10s
#  sleep_duration: 60min
docker run --rm --privileged -v "${PWD}":/config --device=/dev/ttyUSB0 -it ghcr.io/libretiny-eu/libretiny-esphome-docker:latest run smoke.yaml --device /dev/ttyUSB0
INFO ESPHome 2023.9.0-dev
INFO Reading configuration smoke.yaml...
ERROR Unable to import component deep_sleep:
Traceback (most recent call last):
  File "/esphome/esphome/loader.py", line 165, in _lookup_module
    module = importlib.import_module(f"esphome.components.{domain}")
  File "/usr/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 790, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File ".esphome/external_components/36400cb9/esphome/components/deep_sleep/__init__.py", line 23, in <module>
    from esphome.components.esp32.const import (
ImportError: cannot import name 'VARIANT_ESP32C2' from 'esphome.components.esp32.const' (/esphome/esphome/components/esp32/const.py)
Failed config

@Liionboy
Copy link

Liionboy commented Jan 31, 2024

Hello,

Any news on this thread? At me i try on CBU chip and after 1 min wakeup alone without doing something, my code is this for deep sleep:

external_components:
  - source: github://Xmister/libretuya-esphome@deep-sleep
    components: [ deep_sleep ]

bk72xx:
  board: generic-bk7231n-qfn32-tuya
  framework:
    version: dev

deep_sleep:
  id: deep_sleep_control
  sleep_duration: 10min

binary_sensor:
  - platform: homeassistant
    id: prevent_deep_sleep_test
    name: Prevent Deep Sleep
    entity_id: input_boolean.disable_deep_sleep

script:
  - id: consider_deep_sleep
    mode: queued
    then:
      - delay: 30s
      - if:
          condition:
            binary_sensor.is_on: prevent_deep_sleep_test
          then:
            - logger.log: 'Skipping sleep, per prevent_deep_sleep'
          else:
            - deep_sleep.enter: deep_sleep_control
      - script.execute: consider_deep_sleep

Log from esphome:

[22:00:30][D][api:102]: Accepted 192.168.1.78
[22:00:30][D][api.connection:1121]: Home Assistant 2024.1.5 (192.168.1.78): Connected successfully
[22:00:31][D][homeassistant.binary_sensor:026]: 'input_boolean.disable_deep_sleep': Got state OFF
[22:00:31][D][binary_sensor:034]: 'Prevent Deep Sleep': Sending initial state OFF
[22:00:49][I][deep_sleep:136]: Beginning Deep Sleep
[22:00:49][I][deep_sleep:138]: Sleeping for 600000000us
[22:00:49][D][lt.preferences:104]: Saving 1 preferences to flash...
[22:00:49][D][lt.preferences:132]: Saving 1 preferences to flash: 0 cached, 1 written, 0 failed
INFO Processing expected disconnect from ESPHome API for lumina-hol3 @ 192.168.2.64
WARNING Disconnected from API
WARNING Can't connect to ESPHome API for lumina-hol3 @ 192.168.2.64: Timeout while connecting to [AddrInfo(family=<AddressFamily.AF_INET: 2>, type=<SocketKind.SOCK_STREAM: 1>, proto=6, sockaddr=IPv4Sockaddr(address='192.168.2.64', port=6053))] (TimeoutAPIError)
INFO Trying to connect to lumina-hol3 @ 192.168.2.64 in the background
INFO Successfully connected to lumina-hol3 @ 192.168.2.64 in 1.094s
INFO Successful handshake with lumina-hol3 @ 192.168.2.64 in 0.040s
[22:02:44][I][deep_sleep:136]: Beginning Deep Sleep
[22:02:44][I][deep_sleep:138]: Sleeping for 600000000us
[22:02:44][D][lt.preferences:104]: Saving 1 preferences to flash...
[22:02:44][D][lt.preferences:132]: Saving 1 preferences to flash: 0 cached, 1 written, 0 failed
INFO Processing expected disconnect from ESPHome API for lumina-hol3 @ 192.168.2.64
WARNING Disconnected from API

Thanks

@devgs
Copy link
Contributor

devgs commented Mar 31, 2024

It's so unfortunate that BDK ignores gpio_stay_*_map all together and always pulls pins either up or down. This makes it impossible to have a single magnetic sensor setup that only able to pull low, to wake up device on both level changes. Internal pull-down impedance is so low that it doesn't allow voltage to go up in an energy-conserving setup. Using a smaller resistor will defeat the purpose of having an energy efficient deep-sleep device (door sensor).

Can anyone please point me in a direction that will allow me to patch the BDK and still be able to use ESPHome?

@kuba2k2
Copy link
Member

kuba2k2 commented Mar 31, 2024

For now, we do not patch the Beken BDK directly, Instead, "fixups" in the LibreTiny repo are used. See cores/beken-72xx/base/fixups.

This is going to change in the future, where the BDK will be cleaned up and patched accordingly.

@devgs
Copy link
Contributor

devgs commented Mar 31, 2024

Thank you for such a quick reply.

I'm rushing the horses here, of course. But would you mind me making a PR if I end up getting it working?

@kuba2k2
Copy link
Member

kuba2k2 commented Mar 31, 2024

Not at all, feel free 😄 I will gladly accept any valuable contribution to the project 🙂

@ster1um
Copy link

ster1um commented May 27, 2024

it not possible wake up from gpio?

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

Successfully merging this pull request may close these issues.

[BUG/Missing] [BK7231N / CB3S] deep sleep support
8 participants