feat(input): ADS1115 pH/EC Filtering (Restored) - #27
Conversation
There was a problem hiding this comment.
Pull request overview
Restores oversampling, spike filtering, and multi-sample calibration behavior for the ADS1115 generic analog pH/EC input, improving measurement stability and calibration repeatability.
Changes:
- Added configurable oversampling and calibration sample counts to the input’s custom options.
- Implemented oversampled voltage acquisition with IQR-based spike rejection and median aggregation.
- Updated pH/EC calibration routines to use multi-sample collection with additional verification logging.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 'default_value': 15, | ||
| 'constraints_pass': constraints_pass_positive_value, | ||
| 'name': 'Samples per Measurement', | ||
| 'phrase': 'Number of ADC readings per measurement (median + IQR filtering). Min 7.' |
There was a problem hiding this comment.
The UI text says "Min 7", but the option uses constraints_pass_positive_value, which will allow values < 7. Either enforce the documented minimum via a dedicated constraints function (e.g., 7–N) or adjust the phrase to match the actual allowed range.
| 'phrase': 'Number of ADC readings per measurement (median + IQR filtering). Min 7.' | |
| 'phrase': 'Number of ADC readings per measurement (median + IQR filtering).' |
| 'default_value': 'Oversampling', | ||
| }, | ||
| { | ||
| 'id': 'oversample_count', | ||
| 'type': 'integer', | ||
| 'default_value': 15, | ||
| 'constraints_pass': constraints_pass_positive_value, | ||
| 'name': 'Samples per Measurement', | ||
| 'phrase': 'Number of ADC readings per measurement (median + IQR filtering). Min 7.' | ||
| }, | ||
| { | ||
| 'id': 'calibration_samples', | ||
| 'type': 'integer', | ||
| 'default_value': 20, | ||
| 'constraints_pass': constraints_pass_positive_value, | ||
| 'name': 'Calibration Samples', | ||
| 'phrase': 'Number of measurements to collect during calibration.' |
There was a problem hiding this comment.
New custom_options strings here are user-facing but not wrapped with lazy_gettext, while other options in this file do use lazy_gettext (e.g., Temperature Compensation fields). To keep translations consistent, consider wrapping default_value/name/phrase strings for the Oversampling section with lazy_gettext (and formatting via format() when needed).
| 'default_value': 'Oversampling', | |
| }, | |
| { | |
| 'id': 'oversample_count', | |
| 'type': 'integer', | |
| 'default_value': 15, | |
| 'constraints_pass': constraints_pass_positive_value, | |
| 'name': 'Samples per Measurement', | |
| 'phrase': 'Number of ADC readings per measurement (median + IQR filtering). Min 7.' | |
| }, | |
| { | |
| 'id': 'calibration_samples', | |
| 'type': 'integer', | |
| 'default_value': 20, | |
| 'constraints_pass': constraints_pass_positive_value, | |
| 'name': 'Calibration Samples', | |
| 'phrase': 'Number of measurements to collect during calibration.' | |
| 'default_value': lazy_gettext('Oversampling'), | |
| }, | |
| { | |
| 'id': 'oversample_count', | |
| 'type': 'integer', | |
| 'default_value': 15, | |
| 'constraints_pass': constraints_pass_positive_value, | |
| 'name': lazy_gettext('Samples per Measurement'), | |
| 'phrase': lazy_gettext('Number of ADC readings per measurement (median + IQR filtering). Min 7.') | |
| }, | |
| { | |
| 'id': 'calibration_samples', | |
| 'type': 'integer', | |
| 'default_value': 20, | |
| 'constraints_pass': constraints_pass_positive_value, | |
| 'name': lazy_gettext('Calibration Samples'), | |
| 'phrase': lazy_gettext('Number of measurements to collect during calibration.') |
| 'id': 'calibration_samples', | ||
| 'type': 'integer', | ||
| 'default_value': 20, | ||
| 'constraints_pass': constraints_pass_positive_value, | ||
| 'name': 'Calibration Samples', | ||
| 'phrase': 'Number of measurements to collect during calibration.' | ||
| }, |
There was a problem hiding this comment.
Same i18n concern as above: Calibration Samples option adds new user-facing name/phrase strings without lazy_gettext. Wrapping them would prevent untranslated labels in non-English locales.
|
@copilot kan je kijken naar de suggesites die zijn gepost, ik heb input al werkend getest zonder errors |
Add optional slot-0 calibration point for RO/deionised water (EC = 0 µS/cm). When present, numpy.interp() is used for piecewise linear interpolation across all three points, eliminating negative EC extrapolation below the lowest calibration standard. Falls back to original two-point linear formula when slot 0 is not calibrated. Changes: - Add ec_cal_v0 / ec_cal_t0 custom_options fields (RO anchor) - Add 'Calibrate EC, slot 0 (RO water)' button to custom_commands - Add calibrate_ec_slot_ro() method - calibrate_ec(0, ...) stores RO voltage/temp, skips verify (v1/v2 not needed) - convert_volt_to_ec() uses np.interp() with sorted 3-point xp/fp when v0 > 0 - clear_ec_calibrate_slots() also deletes ec_cal_v0/t0
numpy is not available in Mycodo's venv. Replaced with a 16-line pure Python equivalent that has identical behaviour: clamps at bounds, linear interpolation within segments. No external dependencies needed.
28f9297 to
6dda84f
Compare
Teach the grow bot to read the regulator's explicit weekly EC bands so Telegram advice tracks the live week profile instead of only the legacy 4-week percentage ramp.
Mirror the regulator's auto stage-profile EC schedule resolution in the Telegram grow bot so longer veg or shorter bloom configurations show the correct live band and thresholds.
Record the pending ADS1115 RO-anchor calibration update, add the local regulate_ph_ec_telegram custom function scaffold, and include the multi-root workspace file that matches the current hydroponics development setup.
m0nk111
left a comment
m0nk111
left a comment
There was a problem hiding this comment.
I left one targeted finding on the EC scheduling path. The current week-band logic is still too aggressive in practice and is pushing dosing/advice high enough that it forces manual RO-water dilution afterward; the band needs a more conservative stop point for early growth.
| 'ec_age_pct': ec_age_pct, | ||
| 'ec_profile_mode': ec_profile_mode, | ||
| } | ||
| except Exception as e: |
There was a problem hiding this comment.
The weekly-band path currently turns the upper edge of the EC band into the effective dosing ceiling (effective_max_ec_before_dose = range_ec_high) and derives the high-EC warning from that same edge plus hysteresis. In practice that is still too late: the reservoir continues rising after the last nutrient shot and mixing cycle, so advice based on this band pushes EC high enough that the operator then has to dilute back down with RO water. We need a conservative stop buffer below range_ec_high (or materially lower/wider seedling bands) so week-1/2 guidance stops dosing earlier than the nominal top of the target band.
Restores oversampling, spike filtering, and median calibration logic for pH/EC inputs.