bristlemouth / bm_protocol

Primary Bristlemouth firmware repository
https://www.bristlemouth.org/
Apache License 2.0
11 stars 8 forks source link

Sc 201662/turbidity app #133

Closed victorsowa12 closed 5 months ago

victorsowa12 commented 5 months ago

Seapoint turbidity app. Spec sheet: https://www.notion.so/sofarocean/SM-product-spec-Seapoint-STS-Adapter-3f7ada5bb1dc4834875efdd45407d257

printf output from the mote serial

turbidity | tick: 114439, rtc: 2024-04-24T20:35:56.804, line: 122.44,1624.4
turbidity | tick: 115439, rtc: 2024-04-24T20:35:57.804, line: 122.45,1625.0
turbidity | tick: 116439, rtc: 2024-04-24T20:35:58.804, line: 122.46,1628.0
turbidity | tick: 117439, rtc: 2024-04-24T20:35:59.804, line: 122.45,1627.2
turbidity | tick: 118439, rtc: 2024-04-24T20:36:00.804, line: 122.47,1626.7

SENS_AGG.csv output:

cat 0000_SENS_AGG.csv
bm_node_id,node_position,node_app_name,timestamp(ticks/UTC),reading_count
9f04aae3187989c8,1,spt_sts,1713991050.531,143,122.4469,1625.617
9f04aae3187989c8,1,spt_sts,1713991110.546,29,122.4386,1625.490
9f04aae3187989c8,1,spt_sts,1713991170.562,29,122.3955,1625.690
9f04aae3187989c8,1,spt_sts,1713991230.574,29,104.6310,1576.876
9f04aae3187989c8,1,spt_sts,1713991290.589,25,118.5032,1614.920
9f04aae3187989c8,1,spt_sts,1713991350.601,28,118.6650,1615.364
9f04aae3187989c8,1,spt_sts,1713991410.617,28,118.7350,1616.061
9f04aae3187989c8,1,spt_sts,1713991470.632,28,118.8486,1615.943
9f04aae3187989c8,1,spt_sts,1713991530.644,28,118.8904,1616.343
9f04aae3187989c8,1,spt_sts,1713991590.660,28,118.8796,1616.418
9f04aae3187989c8,1,spt_sts,1713991650.675,28,118.8246,1616.004
9f04aae3187989c8,1,spt_sts,1713991710.687,28,118.7693,1616.271
9f04aae3187989c8,1,spt_sts,1713991770.703,28,118.7129,1615.411

SENS_IND.csv output:

9f04aae3187989c8,1,spt_sts,7756,6593470336.000,0.000,118.7100,1615.000
9f04aae3187989c8,1,spt_sts,8756,6593470336.000,0.000,118.7000,1615.200
9f04aae3187989c8,1,spt_sts,9756,1713991871.042,0.000,118.6900,1615.900
9f04aae3187989c8,1,spt_sts,10759,1713991872.042,0.000,118.7100,1613.700
9f04aae3187989c8,1,spt_sts,11759,1713991873.042,0.000,118.6800,1613.700
9f04aae3187989c8,1,spt_sts,12759,1713991874.042,0.000,118.6800,1613.700
9f04aae3187989c8,1,spt_sts,13759,1713991875.042,0.000,118.6900,1614.600
9f04aae3187989c8,1,spt_sts,14759,1713991876.042,0.000,118.6900,1616.000
9f04aae3187989c8,1,spt_sts,15759,1713991877.042,0.000,118.7000,1614.800
9f04aae3187989c8,1,spt_sts,16769,1713991878.054,0.000,118.6900,1618.500
9f04aae3187989c8,1,spt_sts,17759,1713991879.042,0.000,118.6800,1614.000
9f04aae3187989c8,1,spt_sts,18769,1713991880.054,0.000,118.6700,1613.900
9f04aae3187989c8,1,spt_sts,19769,1713991881.054,0.000,118.6800,1615.100
9f04aae3187989c8,1,spt_sts,20769,1713991882.054,0.000,118.6900,1615.000
9f04aae3187989c8,1,spt_sts,21769,1713991883.054,0.000,118.6600,1617.500
victorsowa12 commented 5 months ago

@russelldeguzman I've got an outstanding comment for turbidityReadingPeriodMs in notion. I believe this config is for the bridge not the turbidity app, since it most closely reflects other configs on the bridge. (I left it as STS app scope for the notion table filtered view) I'll triple check with Evan.

I beleive bm_node_id,node_position,node_app_name,timestamp(ticks/UTC),reading_count is the correct header for the SENS_AGG file. the new columns are unlabelled since it can be shared with rbr/soft/aanderaa data. Is there something else that needs to change in the header?

victorsowa12 commented 5 months ago

@russelldeguzman I got some clarification from @evanShap. The turbidityReadingPeriodMs is going to be a variable set by tooling on the mote to document the settings of the turbidity sensor, but not used by the app. So the idea would be if anyone ever manually removes the turbidity sensor and configures it to sample at a different rate, they also need to manually change the config so it is matches.

At the same time a config like turbidityReadingPeriodMs is also needed on the bridge to calculate buffer sizes for its aggregations. Evan and I discussed using the same config name for both. Do you or @towynlin have any thoughts on using the same name for both?

victorsowa12 commented 5 months ago

okay everything previosly named spt_sts (or something similar) should now be renamed to seapoint_turbidity (or similar)

towynlin commented 5 months ago

🤦

I think it makes sense in this case to follow the SOFT example and keep them both named turbidityReadingPeriodMs. 👍

evanShap commented 5 months ago

Functionality looks good. 👍

I believe the app name spt_sts is confusing and would prefer to change it to turbidity or seapoint_turbidity.

Questions about the spec, more for @evanShap:

  1. Do we never write to the sensor?
  2. Do we not need FTL mitigations?
  1. Nope - not for this prototype delivery.
  2. FTL was never observed to be an issue with this sensor in the past. I think it makes sense to eventually promote FTL mitigations to a "Core Module" behavior - any app could have it, and it could be enabled and configured for the particular sensor depending on the use case. That's definitely not needed for this prototype delivery though.