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

Add a notification to charge device when battery is smaller than 20% #1768

Open
kodjima33 opened this issue Feb 8, 2025 · 17 comments · May be fixed by #1797
Open

Add a notification to charge device when battery is smaller than 20% #1768

kodjima33 opened this issue Feb 8, 2025 · 17 comments · May be fixed by #1797

Comments

@kodjima33
Copy link
Collaborator

No description provided.

@Satwikloka
Copy link

Hey @kodjima33 ,
I wanted to work on this issue.

@mdmohsin7
Copy link
Collaborator

@Satwikloka we don’t assign or lock bounties to anyone unless they have previously had a PR merged into our repository. If no one is working on an issue, feel free to drop a comment, start working on it, and open a PR.

@ologbonowiwi
Copy link

how will be the bounty be paid and what is the value? Im not seeing this issue on Algora.

@beastoin
Copy link
Collaborator

beastoin commented Feb 9, 2025

Image Image

https://www.omi.me/bounties

@ologbonowiwi ~

@shreybirmiwal
Copy link
Contributor

Hi, I have done a good bit of code on this:
https://github.com/BasedHardware/omi/compare/main...shreybirmiwal:notify-low-battery?expand=1

I am just stuck on the last part (I left a comment), calling the custom API function in the onbatterylevelchange function.

Anyone have any help to finish this last bit (we can split the bounty?)

@andrewgazelka
Copy link

@addbounty $50

@addbounty
Copy link

addbounty commented Feb 11, 2025

banner
button

Make a Draft PR early so others can see you are working on it! To automatically create one:

# Using npx (installed if you have NodeJS/npm)
npx bountybot start BasedHardware/omi#1768

# Or, use cargo (installed if you have Rust)
cargo install bounty
bounty start BasedHardware/omi#1768

When merged, you will receive the bounty!

@mdmohsin7
Copy link
Collaborator

@shreybirmiwal you don't have to rely on the backend to send notification. You can create notification locally

      NotificationService.instance.createNotification(
        title: 'Your Omi Device Disconnected',
        body: 'Please reconnect to continue using your Omi.',
      );

@joshijoe05
Copy link
Contributor

joshijoe05 commented Feb 11, 2025

/attempt #1768

Hey, @mdmohsin7
Do you mean the app should listen to the battery level of the device and should notify user when the battery level is under 20% ?

@shreybirmiwal
Copy link
Contributor

@mdmohsin7 thank u very much for the help!

PR: #1792

Not 100% sure if it works - not sure how to test

@mdmohsin7
Copy link
Collaborator

/attempt #1768

Hey, @mdmohsin7 Do you mean the app should listen to the battery level of the device and should notify user when the battery level is under 20% ?

Yes! The app already does listen for battery changes. You'll just have to show a notification when charge level is less than 20%

@joshijoe05
Copy link
Contributor

joshijoe05 commented Feb 12, 2025

/attempt #1768
Hey, @mdmohsin7 Do you mean the app should listen to the battery level of the device and should notify user when the battery level is under 20% ?

Yes! The app already does listen for battery changes. You'll just have to show a notification when charge level is less than 20%

I got it now, but i guess its implemented in the PR raised by @shreybirmiwal
But is there a way to test these without devices @mdmohsin7 ?

@mdmohsin7
Copy link
Collaborator

I got it now, but i guess its implemented in the PR raised by @shreybirmiwal

His PR still relies on backend for the notification, you can open a PR as well making use of local notification (which makes more sense in this case)

But is there a way to test these without devices @mdmohsin7 ?

If you don't have the device, then you can kinda mock the service and only check if the app is showing the notification or not and I'll test it with device while reviewing. One thing to note is the notification should only be shown once and the user should not be overwhelmed with multiple notifications (every time the percentage changes from let's say 20 to 19 and then to 18 and so on)

@shreybirmiwal
Copy link
Contributor

I got it now, but i guess its implemented in the PR raised by @shreybirmiwal

His PR still relies on backend for the notification, you can open a PR as well making use of local notification (which makes more sense in this case)

But is there a way to test these without devices @mdmohsin7 ?

If you don't have the device, then you can kinda mock the service and only check if the app is showing the notification or not and I'll test it with device while reviewing. One thing to note is the notification should only be shown once and the user should not be overwhelmed with multiple notifications (every time the percentage changes from let's say 20 to 19 and then to 18 and so on)

@mdmohsin7 I think I deleted the old backend and switched to the notification from front end in my PR update #1792

And I believe I covered the issue of overwealming notifications by only notifying 1x when below 20% battery in the code.

I will try to test this weekend ( I assume that takes building the backend / frontend and that is a lot of work ?)

@bozhevolnaia
Copy link

so on iPhone it is gonna be double notification about the battery approached 20%, because apple sends one?

@mdmohsin7
Copy link
Collaborator

@mdmohsin7 I think I deleted the old backend and switched to the notification from front end in my PR update #1792
And I believe I covered the issue of overwealming notifications by only notifying 1x when below 20% battery in the code.
I will try to test this weekend ( I assume that takes building the backend / frontend and that is a lot of work ?)

Oh! I haven't checked the PR yet. And no you'll only have to build the app, you don't need to build the backend

@mdmohsin7
Copy link
Collaborator

so on iPhone it is gonna be double notification about the battery approached 20%, because apple sends one?

This is for the omi devices battery and not for the iPhone's battery

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Someday
Development

Successfully merging a pull request may close this issue.

10 participants