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 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 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.

This is a cool idea

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 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.

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?

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.

@stoehraj Hi. Could you update this PR, so we can merge it hopefully?

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?

@stoehraj Nah I don't care. Do what you want.

@Riksu9000 -- just took care of all of the squashing. Got everything into one nice commit and (hopefully) ready to merge in.

It is a miniscule issue, but I think it might be a right thing to handle it:
roll = ble_ll_rand() % diceSizes[diceSizeIndex] + 1; introduces so called "modulo bias".
See f.e. https://funloop.org/post/2015-02-27-removing-modulo-bias-redux.html

It is a miniscule issue, but I think it might be a right thing to handle it: roll = ble_ll_rand() % diceSizes[diceSizeIndex] + 1; introduces so called "modulo bias". See f.e. https://funloop.org/post/2015-02-27-removing-modulo-bias-redux.html

@medeyko -- I just implemented this in the latest changes.

Sorry for persisting, but I have a question again: are there any issues left with this app?
It looks well-written and compact, but still useful.

Is it possible to do anything in order to help to include it in the firmware?

@medeyko It's difficult to say, the amount of available RAM and storage is getting very very close. It's difficult to choose which new features should be merged first, if there's even space available.

How to keep track of features that depend on additional resources, or do not change the memory footprint and could be instantly merged, is currently WIP.

Hopefully we'll be able to more clearly communicate what's the status of each PR soon.

@Avamander a bit off-topic to this PR but as a reply to your thoughts: maybe, just maybe, there need to be some straight-forward way of selecting which apps to include in the build or even have "editions/presets"

@Avamander a bit off-topic to this PR but as a reply to your thoughts: maybe, just maybe, there need to be some straight-forward way of selecting which apps to include in the build or even have "editions/presets"

This is something I have in mind : adding the possibility to select which apps will be installed at build time. This is something that is definitely possible to do. However, I think it'll be very difficult to maintain and test : we'll have to maintain and test multiple 'editions', and/or ensure that every combination of apps will work well, ensure that there's enough RAM memory available for all the apps,...

Sorry for asking again, but could anyone please write what is the state of this app?
Is there anything left to do with it before inlcusion?
How much resources does it really need?
How much resources are relly left?
I guess this app is one of the smallest possible apps, as it doesn't use any unusual library functions, which may significantly increase code size. It is compact itself.
But it is still usable and also useful.

Just a little neat addition: I think it would be cool if, when the user has set up the "dice" (that is, the dice number and such), they can shake the PineTime, and it "rolls" the dice.

@Ertain hey, thanks for the comment -- if you look at the original description and some of the earlier comments, that actually is a feature I had initially, but we ended up pulling it out when there was some investigation on standardizing gestures a bit more across apps. I think it's a really cool idea, though! Makes the rolling a bit more tangible, especially combined with vibration.

Either way, looks like there's a PR for a different dice app that seems more likely to get merged in than this one (#1326) so may be worth it to request that feature over there as well.

closing in favor of merged #1326