73 crash on vivoactive 5 after a few minutes operation

[pmithrandir]

Making sure accel data are fully present before sending them to the phone. (prevent null pointer error)

[jones139]

Wow, that was quick, thank you - I am still trying to get the Garmin SDK working on my new laptop!

I think you have based this pull request against the master branch - please will you base it against the develop branch so it does not look like as many changes?

(I just realised that I need to merge our latest develop branch into master, but haven't done it yet, sorry!).

Thanks!

Graham.

On Sun, 8 Sept 2024 at 21:16, Pierre Bonneau @.***> wrote:

@pmithrandir https://github.com/pmithrandir requested your review on:

74 https://github.com/OpenSeizureDetector/Garmin_SD/pull/74 73 crash

on vivoactive 5 after a few minutes operation.

— Reply to this email directly, view it on GitHub https://github.com/OpenSeizureDetector/Garmin_SD/pull/74#event-14174981040, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACLSY2PIH3Q77XLYGXVWNDZVSWDTAVCNFSM6AAAAABN3I3QF6VHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJUGE3TIOJYGEYDIMA . You are receiving this because your review was requested.Message ID: @.***>

-- Graham Jones http://google.com/+GrahamJones Hartlepool, UK.

[pmithrandir]

Done

[pmithrandir]

Hi.I thought that if data were invalid... It was better to ignore them than to work with them.Puting one sec to 0 seemed ok to keep data structure.And knowing that we are on a case that should not happen (SDK should garantee the 25 events to be here) I wouldn't bet that the 3 arrays have the same size.Pierre Sent from Android deviceLe 8 sept. 2024 22:26, Graham Jones @.***> a écrit : @jones139 commented on this pull request.

I see that if we do not receive the expected number of samples, you are setting them all to zero. Why is this? I was thinking more along the lines of making the loop for (var i=0; i<size(accelData.x); i++) That way we would use the data which is sent, and only zero anything missing (or wait for the next update to add the missing data in?)

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

[jones139]

Ah, I see - assume there is something wrong with the whole sample of 25 readings. That sounds ok - I had been working on the assumption that it was just a timing glitch that meant we had one too many or one too few readings. Have you tested this version to confirm it runs ok? If so we could build a test version for the user to try out and see how it works?

On Mon, 9 Sept 2024, 06:17 Pierre Bonneau, @.***> wrote:

Hi.I thought that if data were invalid... It was better to ignore them than to work with them.Puting one sec to 0 seemed ok to keep data structure.And knowing that we are on a case that should not happen (SDK should garantee the 25 events to be here) I wouldn't bet that the 3 arrays have the same size.Pierre Sent from Android deviceLe 8 sept. 2024 22:26, Graham Jones @.***> a écrit : @jones139 commented on this pull request.

I see that if we do not receive the expected number of samples, you are setting them all to zero. Why is this? I was thinking more along the lines of making the loop for (var i=0; i<size(accelData.x); i++) That way we would use the data which is sent, and only zero anything missing (or wait for the next update to add the missing data in?)

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/OpenSeizureDetector/Garmin_SD/pull/74#issuecomment-2337142432, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACLSY34346YIOETDI5XQNLZVUVQBAVCNFSM6AAAAABN3I3QF6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZXGE2DENBTGI . You are receiving this because you were mentioned.Message ID: @.***>

[pmithrandir]

Hi.I run it on my watch yesterday evening. Seemed ok, but I was unable to reproduce the bug. So I can only say it's not adding regression.PierreSent from Android deviceLe 9 sept. 2024 08:39, Graham Jones @.***> a écrit : Ah, I see - assume there is something wrong with the whole sample of 25

readings. That sounds ok - I had been working on the assumption that it

was just a timing glitch that meant we had one too many or one too few

readings.

Have you tested this version to confirm it runs ok? If so we could build

a test version for the user to try out and see how it works?

On Mon, 9 Sept 2024, 06:17 Pierre Bonneau, @.***> wrote:

Hi.I thought that if data were invalid... It was better to ignore them

than to work with them.Puting one sec to 0 seemed ok to keep data

structure.And knowing that we are on a case that should not happen (SDK

should garantee the 25 events to be here) I wouldn't bet that the 3 arrays

have the same size.Pierre Sent from Android deviceLe 8 sept. 2024 22:26,

Graham Jones @.***> a écrit :

@jones139 commented on this pull request.

I see that if we do not receive the expected number of samples, you are

setting them all to zero. Why is this?

I was thinking more along the lines of making the loop

for (var i=0; i<size(accelData.x); i++)

That way we would use the data which is sent, and only zero anything

missing (or wait for the next update to add the missing data in?)

—Reply to this email directly, view it on GitHub, or unsubscribe.You are

receiving this because you were mentioned.Message ID: @.***>

—

Reply to this email directly, view it on GitHub

https://github.com/OpenSeizureDetector/Garmin_SD/pull/74#issuecomment-2337142432,

or unsubscribe

https://github.com/notifications/unsubscribe-auth/AACLSY34346YIOETDI5XQNLZVUVQBAVCNFSM6AAAAABN3I3QF6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZXGE2DENBTGI

.

You are receiving this because you were mentioned.Message ID:

@.***>

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

[jones139]

I borrowed another computer with an older version of Ubuntu linux on it, and have managed to compile it. It compiles and runs ok on my forerunner 245. I have put a compiled version in this branch here: https://github.com/OpenSeizureDetector/Garmin_SD/raw/73-crash-on-vivoactive-5-after-a-few-minutes-operation/build/GarminSD_v2.0.7a.prg

I will ask the user to try it for a few hours then send the log file so we can see what it says.... I'll leave this PR open until we have feedback on it.

[jones139]

The user has confirmed that this has run ok on her watch for several hours, after it was crashing every few minutes, so merging this PR, thank you!