-
Notifications
You must be signed in to change notification settings - Fork 327
Add support for PowerSave 2 command to enable RF only sleep on Beken N/T #1533
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
base: main
Are you sure you want to change the base?
Conversation
maybe submit a post about this too for greater visibility? |
Remember making this suggestion on elektroda, but i forgot about it. OpenBK7231T_App/src/cmnds/cmd_main.c Line 842 in 82ec9c8
And not in .md file, as it is regenerated every several months. |
Very nice, but does it mean that we now have PowerSave 1 and PowerSave 2 with different meanings on different platforms? Maybe it would be better to change the naming, for example, PowerSaveNoRF or PowerSave NoRF (PowerSave 1 = full powersave, PowerSave 0 = disable, PowerSave NoRF = only mcu sleep) |
@NonPIayerCharacter thank you for the guidance regarding the comment... I have updated in cmd_main.c as suggested! @openshwprojects I agree it is not ideal, obviously I did the best I could to add the new function as a minimalistic change whilst remaining non-breaking for the sake of upgrades. I am not sure it is an issue that the PowerSave setting behaves differently on different platforms as I can't imagine configurations would be shared between platforms often. What do you think? |
Is this now approvable? |
Seems like a nice thing to have |
Configurations may not be shared between platforms, but tutorials will be. And users are beginners, that's why I vote for something like "PowerSave RF" and then try to implement it on all platforms. Otherwise we'll just confuse users. If "PowerSave RF" is not present on current platform, do nothing. |
We already have "On LN882H PowerSave 1 = light sleep and Powersave >1 (eg PowerSave 2) = deeper sleep." so I can't accept giving "PowerSave 2" different meaning on Beken, especially if we are going to have the same "rf only powersave" on LN882H in the future. So I suggest "PowerSave RF" Keep all legacy syntax supported, just add check for "PowerSave RF" and if it's supported, do RF powersave for BL0937, if not, ignore |
No description provided.