Select where to show Widget on AOD

[Der-Schubi]

See discussion with @Navid200 here: #1625 (comment)

[Navid200]

Thanks
I will be testing tomorrow. I am currently busy with Google Cloud Nightsocut. But, it is coming to a good break point.
Thanks a lot for doing this.

[Navid200]

Would you please add a screenshot showing the new settings page as well as another showing the page including the setting that will take you to the new page?

[Der-Schubi]

Would you please add a screenshot showing the new settings page as well as another showing the page including the setting that will take you to the new page?

done

[Navid200]

This is really nice. Looking forward to testing it tomorrow. Thanks

[rudysroost]

@Der-Schubi I'm excited about this too, it will eliminate the biggest and possibly only annoyance I experience with xDrip. Will the number not bounce around if two checkboxes are checked as shown in your screenshot?

[Der-Schubi]

@Der-Schubi I'm excited about this too, it will eliminate the biggest and possibly only annoyance I experience with xDrip. Will the number not bounce around if two checkboxes are checked as shown in your screenshot?

No, its like before my change, but the unchecked parts are avoided.
So every minute it will randomize a position within the checked parts of the screen.

[rudysroost]

@Der-Schubi I'm excited about this too, it will eliminate the biggest and possibly only annoyance I experience with xDrip. Will the number not bounce around if two checkboxes are checked as shown in your screenshot?

No, its like before my change, but the unchecked parts are avoided. So every minute it will randomize a position within the checked parts of the screen.

Thanks for the reply, maybe someday we can have an option for a static location like that used on the number wall lock screen configuration. Your work is still a huge improvement for me.

[Navid200]

It's good that selecting all or deselecting all makes it behave as it does now. I will cover this in the guide.

I have Pixel 6a. I have disabled top and bottom center. It avoids the clock at the top and the fingerprint sensor at the bottom center. It is nice.
I see no problem.
Thanks

Let's wait for Jamorham's review.

[Navid200]

Am I just imagining? Or, does this increase the widget's brightness?

[Der-Schubi]

Am I just imagining? Or, does this increase the widget's brightness?

No, but the other one does (#3158).

[Navid200]

@jamorham When you get a chance, please tell us what you may not like about this. Or, what do you think about it?

[Navid200]

Unless you have a concern about this, this should also be merged.

[Der-Schubi]

@jamorham Any thoughts?

[jamorham]

Hi, this looks good and like it should work but the AOD stuff is a bit of a nightmare in that it behaves differently on different devices but additional controls like this have a good chance of improving that compatibility but you have to be very careful not to make assumptions about devices and I have found overall compatibility with AOD has been getting harder so I am very interested in what testing has been done with this feature so far in terms of different devices tested?

[Der-Schubi]

[...] so I am very interested in what testing has been done with this feature so far in terms of different devices tested?

Hi!
I'm running this on vanilla Android (Pixel 5) since over a year (hardcoded, so minus the checkboxes) without problems.
Two friends of mine are running my build with this feature enabled since a month on a Samsung and a Xiaomi device.
Navid wrote he is also using a Pixel (6a).

But I doubt this code can make things worse, as it just narrows down the usable area for the widget.
Up to now we randomize y within the screen height until we find a free spot (loop with 200 tries, see findOverlappingBlock()), my code additionally sorts out unchecked parts (fifths) of the screen height and runs another round in the loop until the randomized y value is free (findOverlappingBlock) and not unchecked in the new settings I added.

[rudysroost]

I would love to test this on my Google Pixel 7 XL, if there is an APK available.

[jamorham]

Hi, I think unit tests are failing. I selected to re-run the test but if they are still failing we need to determine why and whether the specification is still being met or if something else has happened.

[Der-Schubi]

Hi, I think unit tests are failing. I selected to re-run the test but if they are still failing we need to determine why and whether the specification is still being met or if something else has happened.

I changed the tests in this commit: a96921e
I thought this would be enough.
But I'm not able to build right now because of some update, have to fix this first before I can look into it again.

[Navid200]

@Der-Schubi Information is coming tomorrow.

[Der-Schubi]

@Navid200 Up and running again. It was the lombok plugin. Thanks!
@jamorham Could you please trigger a re-run of the unit tests once again? They will pass now.
There was an error in my failsafe and I forgot to remove findRandomAvailablePosition from the tests. I removed this function, there is only findRandomAvailablePositionWithFailSafe now.

[Navid200]

Since there have been changes since I last tested it, I wanted to test it again.
But, I cannot and I am not sure why.

I don't know why I cannot find where this is defined:

https://github.com/Der-Schubi/xDrip/blob/4a9b033b492b029066b47a2014ab090a5fa105da/app/src/main/res/xml/xdrip_plus_prefs.xml#L1236

[jamorham]

@Der-Schubi I get nervous when failing tests are removed in case it is a regression. Can you explain what happened with that method being removed? thanks

[Der-Schubi]

@Der-Schubi I get nervous when failing tests are removed in case it is a regression. Can you explain what happened with that method being removed? thanks

@jamorham There were two functions here. findRandomAvailablePositionWithFailSafe called a second one, named findRandomAvailablePosition. The second one failed with -1 if no free position was found, findRandomAvailablePositionWithFailSafe then returned a random position regardless if free or not.
I merged the two functions together, so findRandomAvailablePositionWithFailSafe will do all the work now and findRandomAvailablePosition was removed. So now the tests are removed too.
These two functions are only used once in the entire code, specifically for the placement of the AOD widget, btw.

@Navid200 Looks totally unrelated too me?! Maybe a merge error on your side? Any news on this?

[jamorham]

Hi, it seems that the unit tests are failing, can you recheck. It reports failed on findRandomAvailablePositionTest

[jamorham]

@Der-Schubi I hope you get a chance to fix these unit tests so we can proceed to merge if all issues are resolved. thanks

[jamorham]

@Der-Schubi please let me know if you get any time to look at this as I would like to get it merged. Thanks

[Der-Schubi]

@jamorham Sorry, I did look into it, but could not get the unit tests to fail on my end. They always pass. So I don't really know whats the problem here.
Is there any output visible here on github where I can check why the tests are failing?

[Navid200]

I will attempt to build this again and see if I can figure out why it failed for me. May be it is related.

[Navid200]

Change of plans:
Since you say you experience something different than we do, I suspect you are not setting up your fork properly. By fork, I mean the repository and branch you import into your Android Studio.

I have a fork. Inside it, I have a branch I have called orig. My orig branch is identical to the official repository branch. I never edit it.

When I want to create a new PR, I first go to my repository and to the orig branch and sync it. That pulls in the latest from the official master branch, which is the latest Nightly, into my orig branch.

Then, I go to the commits in my orig branch, and select that last commit. From that commit, I create a new branch and call it something to let me know what it is. I use a date and I have a log file that shows me what each one of my branches contains.

Then, in Android Studio, I import from GitHub and import from my fork into the same title as my new branch.
After the import is complete, I checkout the new branch in Android Studio.
Now, Android studio project with the new title is associated with the new branch I created in my fork.

I then go ahead and work on my project in Android Studio and make the changes I need and do all my tests. After everything works, I push. This will edit my corresponding GitHub branch from Android Studio.

I then go to my GitHub and open a PR from the updated branch.

I am sorry I don't mean to offend you. This is the only way I know we can figure out why we experience different things.
Can you tell me if you do the same or if you do something else?

What I suspect is that you have some additional edits in your Android Studio that is not pushed into your PR.

[Der-Schubi]

@Navid200
My workflow:
git checkout master
git pull origin master
git checkout --track -b someworkingbranch
Then I open AS, do my changes, build / run tests.
Then I commit the changes, push them to GitHub and create a PR.
Later for changes I pull master again, checkout my workbranch, pull again, which rebases to master and force push.

[Navid200]

I was able to compile your submission with no problem.

May be the problem is that your PR is based on an old release and you should rebase it to see everything that happens when it is applied to the current master.

[Navid200]

When I rebased my version of your PR, I got errors due to mismatches in the strings files both English and German.

[Der-Schubi]

When I rebased my version of your PR, I got errors due to mismatches in the strings files both English and German.

I'll do a rebase tomorrow morning.

[Navid200]

I am testing this now. There is no collision with the latest Nightly any more. Thanks for fixing.

And it looks good.
It's OK to merge.
Of course, as soon as we merge it, someone is going to say "why only 5 regions? Why not 10"?
Or, someone is going to say why isn't the screen also broken down horizontally.

Please leave it as is. Please don't increase the number of regions or the orientation. This is good as is.
Thanks

[jamorham]

@Der-Schubi There is still a failing unit test in com.eveningoutpost.dexdrip.utils.math.BlockFinderTest.findRandomAvailablePositionTest(BlockFinderTest.java:38) which gets a null pointer exception. Are you able to successfully run the tests locally?

[Navid200]

I have no experience with running tests locally. I just ran that test from one of my open PRs and it showed that two tests passed.
But, when I ran it from my local branch equivalent of this PR, one of the two tests failed.

[Der-Schubi]

@Der-Schubi There is still a failing unit test in com.eveningoutpost.dexdrip.utils.math.BlockFinderTest.findRandomAvailablePositionTest(BlockFinderTest.java:38) which gets a null pointer exception. Are you able to successfully run the tests locally?

I was able to run them, but today I tried again and got the same exception. After a "Rebuild Project" I'm able to run the tests without problems again. Cloud this be a cache issue?

[Navid200]

@Der-Schubi This is a real question. I don't know the answer.
Is it possible that the tests need to be updated so that they treat the methods you have modified accordingly?
You have made some changes, right? Are the tests aware of the changes you have made to the files they are testing?
I mean if you have added an argument to an existing method, but, the test is still using it without an argument, could that be the reason the tests fail?

[Der-Schubi]

OK, I had a closer look. The NullPointerException is thrown in line 84 of BlockFinder.java, which is reading the settings: useTop = Pref.getBooleanDefaultFalse("aod_use_top");
I've got no idea why this NPE is not persistent.
But as I already enable all regions if none is enabled in the settings, I just catch the NPE there and set all five to true.
I'll push in a few minutes (still testing). Please run the tests again @jamorham . I hope I solved it with this.

Edit:
Good timing @Navid200
Yes, the tests are aware of all changes.

[Navid200]

After the last change you made, the local tests pass for me.

[jcarrollpe]

Hello, new user here. If/when this is merged will it first appear on the nightly channel? Love the AOD numberwall and would love to see this refinement incorporated. Thanks.

[Der-Schubi]

Yes @jcarrollpe, this will be included in the first nightly build after the merge.

[jamorham]

Thanks for all your work on this

[jcarrollpe]

Works a treat. Thanks everyone!