InfiniTime
InfiniTime copied to clipboard
InfiniTimeOrg
Add dice rolling app
Preview Video
https://user-images.githubusercontent.com/31697004/128671489-f3a153a2-a103-4d30-8fe4-19bd4dd53d4a.mp4
Description
This PR adds a new dice rolling app. I play D&D and thought it would be fun to be able to roll dice on my PineTime, so here we are. I thought others might enjoy it too.
This is what the basic UI looks like:
There are buttons to select the number and size of dice. The number of dice can be between 1 and 99, and the supported dice sizes are d2, d3, d4, d6, d8, d12, d20, and d100.
When the desired number and size of dice have been configured, the "Roll" button will, well, roll the dice. Rolls can also be made by slowly rolling the wrist, as shown below:
https://user-images.githubusercontent.com/31697004/128671511-c4d74205-6235-47f4-ac0b-bc59e43df45a.mp4
If the roll is made using the motion control, the watch will vibrate, giving feedback that the roll occurred.
After rolling, the first line of the output box at the top of the screens shows the total result, and, if multiple dice are rolled, the second line shows each individual die roll.
I might overlook something - but if you use rand() and do not set seed by srand(), do you get the same pseudorandom numbers again after InfiniTime reboots?
I cannot see srand anywhere in the InfiniTime code, so either it should be set system-wide after boot based on NRF52832 RNG peripheral, or in the app where seed could be for example uptime in milliseconds.
Otherwise, nice app. Thanks for adding this feature!
I might overlook something - but if you use
rand()and do not set seed bysrand(), do you get the same pseudorandom numbers again after InfiniTime reboots? I cannot seesrandanywhere in the InfiniTime code, so either it should be set system-wide after boot based on NRF52832 RNG peripheral, or in the app where seed could be for example uptime in milliseconds. Otherwise, nice app. Thanks for adding this feature!
I saw that another app was using rand() (Twos) without anything additional, so I figured everything was all set up for it. I just tested and do indeed get the same output if I do the same roll after a reboot. I'll take care of that.
@hubmartin I could use some help with the srand call for seeding the random number generator. I tried using srand(time(nullptr)); in my object's constructor and also tried doing srand(std::chrono::system_clock::now().time_since_epoch().count()); and neither seemed to have an impact -- that's to say, I could still reproduce getting the same roll each time after a reboot. Would it be better to get the system time using the DateTimeController functions? Also would it be better to have the seed set system-wide as you mentioned? What would that look like?
My guess is that constructors are run immediatelly after start and they are called cycle-perfect every time, so you seed the random, but it is same every time. You have to call srand somewhere where the called function is run based on user's input. It might be Dice::createScreen() which if i'm not mistaken is called when you run application
Then I would suggest to seed rather some milliseconds counter/timer. But your srand(std::chrono::system_clock::now().time_since_epoch().count()); might work, or I see also dateTimeController.Uptime().count() value. Its seconds counter so it might be ok, but I would suggest to find millisecond counter somewhere. Not sure whether it has some function or it needs to be added, because it seems like lot of them returns just a seconds.
With @hubmartin suggestion of using the screen constructor using the current time should be much more effective, dateTimeController.CurrentDateTime().count() should work well as long as the clock is set.
@hubmartin @geekbozu I did a bit more looking at this -- I ended up using srand(dateTimeController.CurrentDateTime().time_since_epoch().count()); in the constructor, and that seemed to do the trick. No repeat rolls after rebooting and trying the same roll (10 six-sided dice) three times.
I believe the constructor is actually called every time the app is opened, rather than once every boot, though with this implementation (using current time rather than uptime) it shouldn't really matter either way.
I gave this a try the other day and it worked really well I will try the new changes and see if it works out well. Never can go wrong with more features.
Is there any plans to actually make apps configurable on compile time so we can remove stuff we don't want when we finally get to the point of running out of room. Not necessarily something you need to figure out just more of a curiosity.
For the seed part for the random number generation, in an embedded software project I participate with we used ADC values on unused pins or very fluctuant voltage in order to have something different each time. I don't know if we have an ADC on the nrf but we can try to use something proportional to the battery voltage for example before filtering if possible.
@toitoinou NRF52 has special and certified RNG peripheral exactly for this reason. I did some quick tests and had something wrong, the numbers did not come. If anyone could look into that it would be great to have system-wide random seed right after boot for the whole system.
Using the RNG is pretty simple on the NRF52 https://devzone.nordicsemi.com/f/nordic-q-a/19429/random-number-without-softdevice. I suggest seeding using srand on boot-up.
If we are going to use the hardware RNG maybe consider making an "RNG" driver so it can maintain portability between different platforms with more "ease".....aka just have it default to std::rand() when not on an NRF platform
@geekbozu we don't need to use RNG for rand. Single RNG number on boot and applying it to the random seed srand should be enough. Then srand gives us number immediatelly. RNG is "slow" if you need many numbers in a row. Maybe could be an issue, maybe not.
This is such a cool idea. Bump, to see this finished and included in the next build.
@geekbozu we don't need to use RNG for
rand. Single RNG number on boot and applying it to the random seedsrandshould be enough. Thensrandgives us number immediatelly. RNG is "slow" if you need many numbers in a row. Maybe could be an issue, maybe not.
Yeah this makes a ton of sense. Even more so for applications where randomness is only partially critical.
This is such a cool idea. Bump, to see this finished and included in the next build.
Hey -- thanks for the support. I'd love to finish this up and get it included in the next build.
Unfortunately life has gotten in the way a bit -- I hopefully should have some time in the next couple weeks to come back to it.
A small recommendation.
When the icon is pressed change the icon to the number that has been rolled , for other things long pressing it can open the full app.
I just pushed new changes that make the dice app compatible with 1.6.0. I likely still have to revisit the tidy warnings, and also the wrist-motion rolling seems to not work on the new version. I'll try to look into these items over the next few days.
The app itself runs fine otherwise.
There are actually two shake detection algorithms in the works. #691 #660. Seems like the roll detection is quite sensitive, so would a shake work better here too?
There are actually two shake detection algorithms in the works. #691 #660. Seems like the roll detection is quite sensitive, so would a shake work better here too?
Oh, that's awesome -- that would be perfect for this application. I'll keep my eyes on that.
@Riksu9000 -- I believe I have all format/tidy issues worked out. At least I didn't see anymore on my end, though I'm not entirely sure I was using clang-tidy correctly. In the meantime, I might just leave this PR open to see if progress gets made on the shake algorithm in the near future. Or do you think it would be better to just strip out my attempt at the motion controls, get the PR pulled in without them, and add motion controls back in later once #691 is pulled in?
Either way, I'd like to squash my commits once the code is finalized and before completing the PR just to keep the git history a bit more clean.
The reason that the roll gesture isn't working is because Refresh() now requires an LVGL task.
This will call the Refresh() function every 20 ticks through the callback function, which is defined in Screen.cpp
taskRefresh = lv_task_create(RefreshTaskCallback, LV_DISP_DEF_REFR_PERIOD, LV_TASK_PRIO_MID, this);
But maybe it's best to remove the gesture and add it separately, as this app works fine without it.
One more thing I noticed while testing is that maybe on pressing the roll button the watch should vibrate or give some other feedback, so we can tell that the tap was actually registered if it ends up rolling the same number.
I would recommend removing the roll and adding it back later. There should be some changes coming down the pipeline on how the BMA interface is defined...one less thing to update with that PR would be nice ;)
@stoehraj Hi, was working on other PR and discovered that Nimble is already using RNG peripheral and it has nice driver and wrapper for rand(). All I had to do was:
#include <controller/ble_ll.h>
and then call uint32_t r = ble_ll_rand(); to get the value.
It seems like Nimble has some buffer of pre-generated values so it seems like you can call ble_ll_rand instead of rand directly without any waiting for RNG to generate numbers.
This also explains why I was not able to use RNG, since Nimble reconfigured registers for its own use.
I think that in the case when n1, d2 the indicator also should write (heads) or (tails) for those who wishes to flip a coin, see #919
@Riksu9000
update this PR, so we can merge it hopefully
As far as I see, @stoehraj doesn't respond. What should be updated in this PR? Maybe it could be done w/o @stoehraj?
@stoehraj Hi. Could you update this PR, so we can merge it hopefully?
@Riksu9000 Hey -- apologies for the extremely late response and lack of activity. I see you've changed some stuff over the past few days. I just flashed the latest stuff onto my unit to play around with a bit.
I do want to try out what @hubmartin suggested with using ble_ll_rand rather than just rand for getting the random values. I'll report back.
Also, if nothing else, I want to squash all of my prior commits to clean up the git history a bit before this gets merged in.
@hubmartin -- the ble_ll_rand() call seems to work really nicely. Just pushed up changes to move to that over the normal rand().
@Riksu9000 -- do you care if I squash your commits in with mine? Or would you prefer to keep yours separate?