Skip to content

Add support for termux-battery-status#1289

Closed
Freed-Wu wants to merge 1 commit intoromkatv:masterfrom
Freed-Wu:iss803
Closed

Add support for termux-battery-status#1289
Freed-Wu wants to merge 1 commit intoromkatv:masterfrom
Freed-Wu:iss803

Conversation

@Freed-Wu
Copy link
Copy Markdown
Contributor

@Freed-Wu Freed-Wu commented Mar 6, 2021

It's a contiuance of #803 (comment)

Thanks for your review!

@Freed-Wu
Copy link
Copy Markdown
Contributor Author

Freed-Wu commented Mar 6, 2021

Screenshot_2021-03-07-03-27-18-57

it can work, but it need more test and improvement.

@Freed-Wu
Copy link
Copy Markdown
Contributor Author

Freed-Wu commented Mar 6, 2021

maybe this line is problematic:

    (( bat_percent = `termux-battery-status|jq -r .percentage` ))

Screenshot-20210307033452-1366x768

this is /data/.profile, when su - u0_a205, this file will be sourced.

	export PREFIX=/data/data/com.termux/files/usr
	export PATH=$PREFIX/bin:$PATH
	export HOME=/data/data/com.termux/files/home
	cd
	exec $PREFIX/bin/zsh

@Freed-Wu
Copy link
Copy Markdown
Contributor Author

Freed-Wu commented Mar 7, 2021

maybe this line is problematic:

    (( bat_percent = `termux-battery-status|jq -r .percentage` ))

it is because termux-battery-status cannot work normally in adb shell.
perhaps should get battery information from acpiclient more priorer than termux-battery-status to solve the problem.

@Freed-Wu
Copy link
Copy Markdown
Contributor Author

Freed-Wu commented Mar 7, 2021

termux api only can work for termux user, not root or shell (adb shell user). so should get battery information from acpiclient prior to termux api.

Now it can work for whoever termux user or root user. the bug that percentage of battery capcity is 0% will be fixed in #1288.

Screenshot_2021-03-07-15-59-22-43

@romkatv
Copy link
Copy Markdown
Owner

romkatv commented Mar 7, 2021

termux-battery-status hangs by default unless Termux:API is installed and given extra permissions. Thus, it's unsafe to invoke it without first ensuring that it won't hang. This is a deal breaker.

In addition, please remove all forks from the code you are adding. The only new fork can be the invocation of termux-battery-status.

@Freed-Wu
Copy link
Copy Markdown
Contributor Author

Freed-Wu commented Aug 7, 2021

i am sorry:(

i asked this question to termux, they added an 'enhance' label for it.
since now, i still don't know how to judge if Termux:API will hang and how to avoid this unsafe invoke.

sorry again.

@Freed-Wu
Copy link
Copy Markdown
Contributor Author

termux-battery-status hangs by default unless Termux:API is installed

Can we add a variable, when user make sure they have installed Termux:API, they set it, p10k will use it to display battery info? Because this variable if unset by default, it will effect other users.

@romkatv
Copy link
Copy Markdown
Owner

romkatv commented Feb 27, 2024

Can you elaborate why you want this segment visible? Doesn't your device already show battery level at the top?

@Freed-Wu
Copy link
Copy Markdown
Contributor Author

When user input command line in shell, the most of attention will be nearby the cursor, which is after shell prompt, not the top.

@romkatv
Copy link
Copy Markdown
Owner

romkatv commented Feb 27, 2024

Thank you for explaining the motivation for displaying batter charge level in the shell prompt when the same information is available at the top of the screen.

The reason you've provided appears to me as a desirable feature rather than an issue issue to be solved or worked around: battery level changes slowly, so there is no need for the battery indicator to insert itself into the active working area of an app.

However, if you feel like you need this, you can send me a clean PR (no unnecessary forks, correct and complete code that follows the local style, etc.), and I'll merge it. I don't recommend it but it's an option.

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.

2 participants