durka / HallMonitor

clean room Galaxy S4 View Cover driver
Apache License 2.0
45 stars 21 forks source link

General Discussion #10

Open Wallace0 opened 10 years ago

Wallace0 commented 10 years ago

Thought we could do with a place for general comments. Can also use this to agree division of labour etc.

So, it looks like you are getting busy with the notifications piece and the telephone answering bit.

To my mind that leaves the media player controls bit, and the alarm clock snooze. I think I'll get cracking with the media player bit first. I'll create a new branch for that off from Master - hopefully we won't have too much fun dealing with merge conflicts when we both merge back in!

Wallace0 commented 10 years ago

Wow, that was easy - I've added in the media player capabilities - you can now also optionally select a media player widget and it will show that if either headphones are plugged in OR there is media already playing, otherwise it will show the default widget / default ugly clock. Along the way I did a couple of tweaks - I re-ordered stuff in the Actions open_cover and close_cover methods - it makes sense to do the screen based activities first. I also shifted the logic for adding the widgets into onResume - as we're now using moveToBack rather than finish() to get DefaultAcitivity and the Configuration activity out of the way then that's the best place for it. I'm guessing that will cause some minor issues merging with your notifications stuff though.

I've also deleted those buttonBarStyle and buttonBarButtonStyle entries in values/styles.xml and values-v11/styles.xml.

Oh, and I've realised I made a mistake calling it DefautActivity - I had thought we might have several different activities, but it's much easier just to have one and swap out the widgets / display items as we need. We should probably refactor and name it something more sensible (e.g. WindowedActivity).

I'll look at the alarm clock controls next - somehow doubt it will be as easy as the media player stuff (which I literally just coded and it worked first time - when does that ever happen!! :0)

durka commented 10 years ago

Sounds good. Yeah, refactoring should come eventually. I re-jiggered the DefaultActivity layout a bit in the notifications branch so that when the layout is deleted in order to swap in the widget, there is something left to put the notifications in. Of course if there don't happen to be any notifications, it could be shrunk to zero height.

Also I saw your note in the code about turning the screen off instead of calling lockNow. I actually did research this and IIRC my conclusion was that there is nothing to do except lockNow. There's no "inverse" to a wakelock. You can turn the screen brightness all the way down, but that isn't the same as turning off the screen. The only other thing I could think to do would be to fake a press of the power button, if that's possible, but I think that's the same as calling lockNow anyway.

VolMi commented 10 years ago

You might be interested to hear that the 0.0.2a.apk works relatively well on my S4 mini with CM 10.2.

"relatively well" means that it works roughly 30% of the time and that it did not kill any kittens, yet. :-) So, most of the time, nothing happens when I open/close my S-view cover, but sometimes screen on/off does work.

Thanks for your efforts, I am quite excited as to how you continue to improve it.

durka commented 10 years ago

Hey, thanks for the report. Feel free to open a separate issue about the S4 mini, though of course the two current contributors don't have one of those so our debugging abilities are limited :). At some point (when we are not furiously adding features) I might ask you for some debugging logs. One question is, are you closing the cover slowly? If you sort of leisurely flip it closed, the proximity sensor can fire before the cover is closed far enough to trigger the other sensor, and so nothing happens. (I've been meaning to try and alleviate this with a timer or something.)

VolMi commented 10 years ago

I already tried simple things like faster/slower (un-)covering, doing it the dark or bright light, after bootup or longer periods, etc...

I am not in hurry, so I will just try new compiled versions as you offer them and see if it helps me. And when it still not works perfectly and I see you slowing down, I can still bother you. Hey, I realize that your are in a furious development mode now. Please enjoy it! :-)

Just make it round on your S4s; I tend to assume that our sensors do not really differ much.

Edit: Ooops, sorry. Opening slowly does actually help a lot. But on closing I have a much higher miss-rate.

Wallace0 commented 10 years ago

Interesting - for me it's working about 80-90% on my S4, but as Alex says it seems to be dependent on the speed of closing the cover. If it's only 30% on the S4 mini then maybe there's a slight difference in how the proximity sensor is configured. It ought to be pretty easy to improve it using a timer to delay the check for the hall sensor after the proximity sensor fires. I'll look into it.

Ref "Also I saw your note in the code about turning the screen off instead of calling lockNow" then that's about my conclusion too. I really don't like just dimming the screen, though faking a power button press sounds an intriguing possibility (though I'd guess we'll hit the same must be a system app roadblock). I like having a delay between screen off and locking my phone in case I change my mind (happens a lot).

Wallace0 commented 10 years ago

OK - think I've fixed it - I've put in the delay as suggested and on a quick test on my S4 it's working 100% of the time now.

@VolMi - if you are feeling really brave you can take the HallMonitor.apk from the bin directory of the mediacontrols branch. That's got this fix in it, but is relatively untested. I will however 100% guarantee that no kittens were harmed in the production of it! ;0)

If you do try it out then please let us know if it worked or not.

VolMi commented 10 years ago

\o/ Wallace, that version is much more reliable. For opening, I am now at a hit rate somewehere in the higher nineties (%). Calmly opening it seems to work always. If I open as fast as I can, screen on is mostly not working, but for any realworld usage, it is really good.

Closing is still a bit problematic. Because the cover flips automatically, it is pretty fast when it hits the screen. I have misses much more often then.

Note that your widget is well centered in my S-view window. That light-blue background is a bit crappy (especially on an OLED), but I understand that this is more a temporary thing.

durka commented 10 years ago

Yeah the color picker is coming soon :p. I'm trying to wrap my head around the closing part. It must be that the hall effect sensor is slow to fire. Or maybe the proximity sensor is unreliable. Unclear.

Wallace0 commented 10 years ago

Lol, yep, that blue colour is pretty bad!

Hmmmm..... interesting, so that's a big improvement on 30% - lemme add another event refire at 500ms and see if that helps some more...

Wallace0 commented 10 years ago

...Ok, that's up there - get it from the same place as before. If it is just a delay thing this should improve it further.

I've got plans for the weekend, so that'll be me done until Monday :0)

Wallace0 commented 10 years ago

The media controls stuff seems to be working fine, so I think that should get merged back into the master branch now. Let me know if you want me to do that.

I've done some digging around the alarm answer stuff and it's totally do-able. I've created an alarm-answer branch from mediacontrols that I'll use to drop that in.

Wallace0 commented 10 years ago

@Alex,

I've just merged all of the changes in the mediacontrols / alarm-answer branches into everything in the notifications branch, into a new branch called 'merge'. I'll continue to work in sub-branches from merge. I've also deleted the mediacontrols and alarm-answer branches as they aren't needed any more.

If you are happy to then you can simply merge the 'merge' branch into master (it should go in with no conflicts) and then use that as the basis to continue with the notifications stuff. That way you won't then have any conflicts with anything else that you do. (or similarly just continue with the notifications piece in a new sub-branch from 'merge').

If not and you just want to continue in the current Notifications branch then you'll have to deal with the conflicts when you do try and merge back in with what's now in the 'merge' branch.

durka commented 10 years ago

Excellent. Thank you. I'm going to try and bring master+notifications up to speed, then, tomorrow (today). This week is totally crazy; on Sunday things return to "normal".

On Thursday, December 5, 2013 at 7:25 AM, Wallace0 wrote:

@Alex (https://github.com/Alex), I've just merged all of the changes in the mediacontrols / alarm-answer branches into everything in the notifications branch, into a new branch called 'merge'. I'll continue to work in sub-branches from merge. I've also deleted the mediacontrols and alarm-answer branches as they aren't needed any more. If you are happy to then you can simply merge the 'merge' branch into master (it should go in with no conflicts) and then use that as the basis to continue with the notifications stuff. That way you won't then have any conflicts with anything else that you do. (or similarly just continue with the notifications piece in a new sub-branch from 'merge'). If not and you just want to continue in the current Notifications branch then you'll have to deal with the conflicts when you do try and merge back in with what's now in the 'merge' branch.

— Reply to this email directly or view it on GitHub (https://github.com/durka/HallMonitor/issues/10#issuecomment-29893388).

Wallace0 commented 10 years ago

Cool. FYI I've found a weird bug in the notifications code - seems that if the app has been cleared from memory (think that normally happens about 30mins after it hasn't been activated) then it falls over when it tries to come up, throwing a null pointer exception in DefaultActivity.onStart whilst doing: grid.setNumColumns(notifs.length);, which can only be if final GridView grid = (GridView)findViewById(R.id.default_icon_container); is coming back as null.

I'm not really sure how that can happen as obviously it's just accessing a configured resource. I'm trialling moving the call to get the view up into the end of the onCreate method - that seems to be working (though it's a hard thing to be sure of in practice). I'll keep testing it today and if that has stabilised it then I'll commit the change into the tidyup branch for you to look at.

Wallace0 commented 10 years ago

....OK, think that fixed it - the changes are committed to tidy-up branch. I also had to move a lot of other calls to findViewById to the onCreate method as they had the same problem. The app seems to be stable now but I'll continue to test it out.

durka commented 10 years ago

Okay, I finally merged merge (heh) into master. I'll work now on fixing bugs in the notifications and on call answering. I noticed what may have been a bug in the alarm screen: if the alarm goes off, but you open the cover instead of using Hall Monitor to dismiss it, then the next time Hall Monitor comes up it's stuck on the alarm view. I haven't looked into that yet.

Wallace0 commented 10 years ago

OK, cool - I'm still slammed and have a busy weekend planned, but I should be able to look at more stuff from Monday onwards. Let me know if you do fix that alarm bug before then, or otherwise I'll look into it on Monday (ish!).

Wallace0 commented 10 years ago

That alarm issue is fixed in the tidy-up branch.

durka commented 10 years ago

Phone calls are now answerable!

durka commented 10 years ago

@Wallace0 I changed the preference system a little. I didn't like all those booleans floating around Functions.Events and such. In my view, the SharedPreferences object that Android manages for us should be the authority on the current state of things. It's automatically persisted between app restarts/reinstalls, which is good. And I set it up to refuse to set the root preference if su -c whoami doesn't work, so remembering whether we have root isn't an issue.

The new system seems to work (testing phone controls, I must have called myself hundreds of times from Gmail by now :) Hopefully I didn't break anything, but let me know.

VolMi commented 10 years ago

Would you mind keeping the binary updated, too? I am a bit reluctant to set up this Eclipse monster...

durka commented 10 years ago

Good call. I'll commit the binary, but beware, it's definitely alpha quality!

On December 19, 2013 at 7:22:44 PM, VolMi (notifications@github.com) wrote:

Would you mind keeping the binary updated, too? I am a bit reluctant to set up this Eclipse monster...

— Reply to this email directly or view it on GitHub.

VolMi commented 10 years ago

Ha, the phone call interaction is seriously cool! The screen is very sensitive, totally nice. Finally, I can use the phone without fiddling with the open cover and smearing the display. :-) But it is important to have the default lock screen disabled. Otherwise, I need to press the power button twice (on→off→on) to bring up HallMonitor's notification.

durka commented 10 years ago

Not sure exactly what you mean so I'm moving it into a separate issue.

durka commented 10 years ago

We have an entry in F-Droid (see the readme for a link). Yay, legitimacy!