search

GameWithPixels / DiceFirmware

Pixels dice firmware code.
MIT License
15 stars 1 forks source link

Only report valid face index in RollState message #45

Closed obasille closed 2 years ago

obasille commented 2 years ago

determineFace() can return -1

The code reading this value should ignore it when its 255. There is the issue of the initial value for the face index, we can arbitrarily decide it's 0.

obasille commented 2 years ago

I got the issue after adding logs to the AccHandler() function. I might have been using deferred logs (set NRF_LOG_DEFERRED to 1 in sdk_config_logging.h)

obasille commented 2 years ago

As suggested, check delta time in AccHandler and log a warning if too low (see #NRF_LOG_ENABLED to do something only when logs are enabled)

obasille commented 2 years ago

The only possibility to have face = 255 is for determineFace() to return -1. And that can only happen if the settings->normals are corrupted.

Also determineFace() should init bestFace with face instead of -1.

obasille commented 2 years ago

determineFace() should also check if accMag > 0 when comparing if accMag < settings->fallingThreshold to avoid dividing by 0

David-Chernis commented 2 years ago

So I implemented the changes and spent an hour just trying to get the situation that you mentioned where Im supposed to get a 255 somewhere for face (or bestFace) in the accHandler, but I still couldnt get determineFace to actually return a 255, I tried with deferred logs on and off, but that didnt seem to change much for me, I also checked inside of determineFace, cause why not, but I never seemed to get the situation.

obasille commented 2 years ago

I can't reproduce the bug either. This is bad news since it means it happens randomly or on some very specific conditions. It must be the calibrated face normals from the settings that get corrupted somehow... At least determineFace() can be fixed so it never returns -1 even when it gets corrupted data.