fix: ensure current meter direction sample members in valid range

[towynlin]

Yep, you've got the intent there. @evanShap what range do you think is permissible to be translated vs what should be considered invalid data? Despite what the info sheet says, we've seen negative values for direction, which is why this was queued up.

[evanShap]

Yep, you've got the intent there. @evanShap what range do you think is permissible to be translated vs what should be considered invalid data? Despite what the info sheet says, we've seen negative values for direction, which is why this was queued up.

ooo - good question. Does the Aanderaa datasheet spec 0 to 360, or might direction be -180 to 180?

• If the Aanderaa datasheet/command reference says Direction data should be in [0, 360), but we're seeing negative directions, we should probably reach out to them for technical support.

In the absence of that, If I had to guess and make a call, I would probably:

• allow us to parse any numeric Direction for logging without any re-mapping in the readings log. This will let us know if we're seeing directions outside of an expected range for follow-up tech support later.
• allow us to re-map data Direction readings from [-360, 360] to [0, 2Pi) for use in the aggregator, and have an encodable range of [0, 2Pi) in the report. If we get bad readings sometimes, and we have to infer those based on a negative Direction, it'll take some work and testing to implement. If Direction can properly rollover to < 0º, we can implement that happy path pretty easily in the aggregator for now.

[towynlin]

If the Aanderaa datasheet/command reference says Direction data should be in [0, 360), but we're seeing negative directions, we should probably reach out to them for technical support.

I believe we are. In last night's soak, 0001_AAND_AGG.csv had content like this:

343a0f3044f7c28,24.958,27.644,-1.438,0.918,19.753
343a0f3044f7c28,27.750,32.270,-1.363,0.954,19.749
343a0f3044f7c28,29.960,30.365,-1.943,0.927,19.806

where that middle -1.438 et al are the direction mean — the other possibility could be that the sensor isn't giving us negative values, but it somehow comes out that way in the aggregation? I'll look over that code path to confirm.

Understood that if we do want to do something like this PR, we should not rotate down if > 2π, and that it's ok to rotate up past zero for values down to -2π if those seem valid, but not if the value is lower. 👌

[towynlin]

Looking at 0001_AAND_IND.csv I only see direction readings in the 0-360 range, so something else is causing the negative values. Maybe we're grabbing the wrong field, for example the next column in the csv is east_cm_s which is often negative in this file. Continuing to dig.

[towynlin]

My current hypothesis is that AveragingSampler::getCircularMean is what gives us negative values, and that they're probably totally valid. Need to verify though.

[towynlin]

Hm no it seems like that function should always give us a positive result. Still digging. @russelldeguzman please go ahead and start today's soak without this PR.

[russelldeguzman]

I did a quick python test with the first 10 values of the 0000_AAND_IND.csv:

>>> a2 = numpy.deg2rad(numpy.array([232.203, 227.507,317.123,317.778,226.359,118.473,295.796,241.099,133.762,205.290]))
>>> print(scipy.stats.circmean(a2))
4.154224050865583

Running the same array through the unit tests we get: -2.1289612563140037, which is 4.15422405087, when you rotate it +2pi.

Does the arm-none-eabi-gcc flavor of math.h:atan2 do the same rotation as scipy.stats.circmean, which is why we expect it to be a positive result?

[towynlin]

:clap: :clap: :clap: Bravo @russelldeguzman!

I don't know, but that's good evidence for going with Evan's guess (valid down to -2π, invalid beyond). I'll make those changes today.

And to give more info, I see that in arm-none-eabi's cmath, atan2 resolves to __builtin_atan2l which I'm guessing gets defined to newlib, for example https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libm/math/e_atan2.c;h=8e9650f290de37a0800f0b822ef960bccf847a55;hb=HEAD — but I can't easily tell the answer to your rotation question. :shrug:

[russelldeguzman]

Wow that's atan2 implementaiton is a midterm question if I ever saw one 😆