search

ponewheel / android-ponewheel

pOneWheel Android app
MIT License
71 stars 25 forks source link

getStatusMode in sharedPreferences #105

Closed biell closed 4 years ago

biell commented 4 years ago

What is the reason for putting getStatusMode() in sharedPreferences? It is wrong on startup (still set to 2) sometimes, like if the app crashed. I think it would be better to store this under BluetoothUtilImpl, then have MainActivity access it via getBluetoothUtil().getStatusMode(), or something else like that.

I am adding a periodic characteristic walk. I want it for battery temperature, but I also thought other things might want it. In doing so, I noticed a number of issues with other scheduled tasks. Most notably the sendKeyChallengeForGemini, a new one is registered for each disconnect and reconnect. I have had 5 of them going at any giving time, firing off 5 challenges every 15 seconds. I can easily fix that by moving the Handler to onCreate in MainActivity and check getStatusMode() == 2 first before issuing the challenge. That is how I came across this. And, in working on it, I have eliminated some startup issues and some crashing issues.

If you see no objections, I will remove getStatusMode and setStatusMode from sharedPreferences and move that functionality into BluetoothUtill / BluetoothUtilImpl.

Since battery temperature is only reported on 5°C changes, I don't want to catch it only when it comes in. When it flips, it is enough of a difference that my temperature based recalculations can show the battery remaining change quickly up and down. By pulling the temperature every 1 or 2 minutes, I can smooth that out. It also seemed like a good frequency to pull last error code.

kwatkins commented 4 years ago

@biell some of that is just leftover messiness... feel free to clean it up, all of it! this is all very much appreciated. i'll also add you to the developer list in order to speed up your changes.

biell commented 4 years ago

@kwatkins I was testing my updates here against the latest changes to master, and the NotificationCompat alert is beeping on every % change for me. Is that the expected outcome? Also, the vibrations aren't working for me for 75%, 50%, ... Do they work for you?

I was able to configure the notifications so that only the 75%, 50%, ... beep. But still not get vibrations to work. Would you be interested in me incorporating the update to keep every percentage change from beeping?

kwatkins commented 4 years ago

I noticed that too... no, it was something I was going to look into - if you can fix it go for it. We can drop the vibration feature for now.

biell commented 4 years ago

I think I have this all worked out. I just need to test some, and then I will open a pull request. If you want to see where I am now, you can look:

https://github.com/ponewheel/android-ponewheel/compare/master...biell:battery_v4

This moves statusMode out of sharedPreferences, cleans up the loopers and creates a periodic bluetooth query, and cleans up notifications. I have vibration working, and each notification is its own entity, so you can use Android notification settings to change the sound or other settings for a specific notification. For example, I opened the Android notification manager settings and set the 50% notification to use the Onewheel App's turnaround warning sound.

Sorry it is taking so long, but it took a bit of work to find the right way I wanted to update the notifications, and I was really busy this last month.