Skip to content

feat: unified data validation framework for EEBUS use cases#181

Open
DerAndereAndi wants to merge 10 commits into
devfrom
feature/improve-data
Open

feat: unified data validation framework for EEBUS use cases#181
DerAndereAndi wants to merge 10 commits into
devfrom
feature/improve-data

Conversation

@DerAndereAndi
Copy link
Copy Markdown
Member

@DerAndereAndi DerAndereAndi commented Jul 12, 2025 •
edited
Loading

Summary

Introduces a generic validation framework and applies it to the EG LPC, MA MGCP, and MA MPC use cases, so data returned by the public API methods is verified against EEBUS spec requirements before it reaches consumers.

The bug-fix portion of the original PR (DeviceConfiguration operator-precedence fix + event-firing consistency across the affected handlers) has been split into #215 and should land first.

What's included

Framework (usecases/internal/)

  • validation.go — generic Validator[T] / BaseValidator[T] with composable rules (RequireField, RequireScaledNumber, ValidateRange, ValidateMinMax, ValidateEnum, ValidateSliceNotEmpty, CombineRules, ConditionalRule, ValidateCustom).
  • measurement.go — MeasurementValidator with measurement-specific rules (RequireMeasurementId, RequireMeasurementValue, RequireValueType, RequireValueSource, RequireValueSourceMandatory, ValidateValueState), plus SkipValueState implementing MGCP-003 ("values with state error or outOfRange SHALL be ignored").
  • MeasurementPhaseSpecificDataForFilter now takes a validator and skips individual failing measurements instead of aborting the whole call.
  • VALIDATION.md — developer guide with usage examples.

Use-case validators

  • EG LPC — LoadControlLimitData, DeviceConfigurationKeyValue, ElectricalConnectionCharacteristic validators; wired into ConsumptionLimit, FailsafeConsumptionActivePowerLimit, ConsumptionNominalMax.
  • MA MGCP — scenario-specific validators for Power, Energy, Current, Voltage, Frequency (incl. MGCP-003 skip behavior).
  • MA MPC — scenario-specific validators for Power, Energy, Current, Voltage, Frequency.

Behavior changes

  • MPC/MGCP: when no measurement passes validation, both now consistently return api.ErrDataNotAvailable (previously divergent between the two use cases).
  • MPC: drops the 45–65 Hz frequency-range check — not part of the MPC spec.

Notes

@DerAndereAndi
Copy link
Copy Markdown
Member Author

DerAndereAndi commented Jul 12, 2025

@sthelen-enqs this should also help for the scenario in your PR enbility/spine-go#52

@sthelen-enqs sthelen-enqs self-requested a review October 22, 2025 12:54
sthelen-enqs
sthelen-enqs requested changes Dec 12, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@sthelen-enqs sthelen-enqs left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

I haven't fully checked through the whole PR, but found a couple of minor nits in the tests that might be nice to fix. I'll check over the rest of the validation later.

general nit: could you run gofmt on the test cases as well?

Comment thread usecases/eg/lpc/public_test.go
Comment thread usecases/eg/lpc/public_test.go
Comment thread usecases/eg/lpc/public_test.go
sthelen-enqs
sthelen-enqs requested changes Dec 12, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@sthelen-enqs sthelen-enqs left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to admit I'm not in love with these changes. The validator logic feels more complicated than necessary, and I fear will make it easier to filter data incorrectly as evidenced by the nits and spec inconsistencies I've found so far.
I, however, do not have a better idea as of right now either.

Comment thread usecases/eg/lpc/public.go Outdated
Comment thread usecases/eg/lpc/public.go Outdated
Comment thread usecases/eg/lpc/public.go Outdated
Comment thread usecases/internal/measurement.go Outdated
Comment thread usecases/internal/measurement.go Outdated
Comment thread usecases/ma/mpc/validators.go Outdated
Comment thread usecases/ma/mpc/validators.go Outdated
Comment thread usecases/ma/mpc/validators.go Outdated
Comment thread usecases/ma/mpc/validators.go Outdated
Comment thread usecases/ma/mpc/validators.go Outdated
@DerAndereAndi
🐛 fix: ensure events only fire when public methods return valid data
f22d8e2 
- Fix DeviceConfiguration OR/AND precedence bug in CheckEventPayloadDataForFilter
  (item.KeyId match was OR'd with item.Value != nil instead of AND'd, causing
  the filter to accept any keyvalue item with a value regardless of KeyId)
- Gate 11 event handlers across EG LPC, MA MGCP, and MA MPC on a successful
  public-method call so event notifications stay consistent with the data
  consumers can actually retrieve via the public API
- Update event tests accordingly
@DerAndereAndi DerAndereAndi changed the title 🐛 fix: ensure events only fire when public methods return valid data feat: unified data validation framework for EEBUS use cases Apr 16, 2026
@DerAndereAndi
feat: add comprehensive validation system for EEBUS data types
ef4066c 
Implement type-safe validation framework using Go generics:

- Add generic validation system in validation.go with BaseValidator
- Support for any SPINE data type with compile-time type safety
- Comprehensive validation rules: RequireField, ValidateRange, ValidateEnum, etc.
- Measurement-specific validation with predefined validators
- Replace MeasurementWithMandatoryData with new validation approach
- Complete GoDoc documentation with usage examples
- Comprehensive test coverage for all validation scenarios
- Markdown documentation guide for developers

Benefits:
- No reflection, direct field access for performance
- Type-safe validators prevent runtime errors
- Composable rules for complex validation scenarios
- Clear error messages with validator context
- Easy extension for new SPINE data types
@DerAndereAndi
refactor: implement unified validation framework for EEBUS measurements
204367a 
- Create generic validation framework with Validator[T] interface
- Implement BaseValidator with composable validation rules
- Add MeasurementValidator for type-safe measurement validation
- Unify validation through MeasurementPhaseSpecificDataForFilter
- Remove non-spec frequency range validation (45-65Hz)
- Add comprehensive test coverage for all validators
- Achieve 100% test coverage for MPC use case

The new validation system ensures spec compliance by:
- Rejecting averageValue types when value is required
- Rejecting empirical sources for energy measurements
- Allowing any ValueState for current measurements
- Validating voltage range (0-1000V) per spec
- Removing frequency range check not in MPC spec

This unified approach reduces code duplication and makes it easier
to maintain consistent validation across all EEBUS use cases.
@DerAndereAndi
♻️ refactor: standardize measurement validation behavior and fix ptr …
fab80c1 
…function duplication

- Replace local ptr[T any] functions with util.Ptr from spine-go utils package
- Standardize "no valid data found" behavior to return errors consistently for both MPC and MGCP
- Update MGCP test expectations to match consistent error handling
- Remove duplicate utility functions across validator test files
- Ensure both use cases handle data availability scenarios uniformly
@DerAndereAndi
✨ feat(eg/lpc): add data validation for EG LPC use case
368856c 
Add validators for EG LPC (Energy Guard Load Power Consumption) use case to validate
data structures when reading from Controllable Systems:

- LoadControlLimitData: validate required fields (LimitId, Value)
- DeviceConfigurationKeyValueData: validate required fields and value types
- ElectricalConnectionCharacteristicData: validate required fields and characteristic types

Validation follows EEBus LPC specification principles:
- Only validate structural integrity and type requirements
- Trust that CS has validated its own data according to spec
- No arbitrary range validations beyond spec requirements

This ensures data integrity when EG reads from CS devices while following the
unified validation framework pattern established in MGCP and MPC use cases.
@DerAndereAndi
🧹 refactor: rename parameters to avoid shadowing built-in min/max fun…
f529f91 
…ctions

- Rename 'min' and 'max' parameters to 'minVal' and 'maxVal' in validation functions
- Fix CodeFactor warnings about redefinition of built-in functions
- No functional changes, improves code maintainability
@DerAndereAndi
✅ test: improve test coverage across multiple use case packages
53be06c 
- Increased internal/measurement.go coverage from 69% to 94.1%
- Enhanced eg/lpc package coverage from 94.7% to 95.2%
- Boosted ma/mgcp package coverage from 94.7% to 97.3%
- Achieved 100% coverage for ma/mpc package (up from 97.4%)

Added comprehensive test cases for:
- Measurement validation functions and edge cases
- LPC validator functions and configuration handling
- MGCP/MPC event callbacks for phase-specific measurements
- Error conditions and invalid data scenarios
@DerAndereAndi
fix(eg/lpc): adapt tests to EntityType-based characteristicType
a62f16f 
- Test_characteristicType_EdgeCases: mock EntityType() instead of Device()/DeviceType(),
  reflecting the CEM-based check introduced on dev (#191).
- Test_ConsumptionNominalMax_ValidationErrors: use PowerConsumptionNominalMax for the
  monitored EVSE entity instead of ContractualConsumptionNominalMax.
- Test_ConsumptionNominalMax_ErrorCases: correct the missing-CharacteristicId assertion
  from ErrDataNotAvailable to ErrDataInvalid (validator catches it; filter still matches).

Also apply gofmt across the branch.
@DerAndereAndi
Address review feedback from @sthelen-enqs
ef77ce3 
Spec correctness

- MPC: add MPC-003 SkipValueState to all scenario validators (Power,
  Energy, Current, Voltage, Frequency) so measurements with
  valueState=error or outOfRange are ignored.
- MPC & MGCP: make valueSource Mandatory (previously Recommended for
  most scenarios), using the same measuredValue/calculatedValue/
  empiricalValue source list for every scenario.
- MPC: drop the ad-hoc 0-1000V voltage range check (not in spec).

API simplifications

- Pass MeasurementValidator by value, not pointer, so nil is a type
  error rather than a runtime check.
- Rename BaseValidator.ValidateFirst -> FindFirstValidItem: the name
  now describes intent rather than mechanism.
- ValidateEnum and RequireValueSource panic when called with an empty
  allowed list (always a programming error) instead of silently
  no-op'ing; drop the parallel RequireValueSourceMandatory variant.
- Use slices.Contains in RequireValueSource.

Dead code

- Delete the legacy Power/Energy/Current/Voltage/Frequency measurement
  validators in usecases/internal/measurement.go (marked deprecated,
  never referenced outside docs).
- Delete the duplicate MGCP*Validator vars in internal/measurement.go
  that shadowed the canonical ones in usecases/ma/mgcp/validators.go.
- Delete basicMGCPValidator (unused) and the local SkipValueState /
  RequireValueSourceMGCP wrappers in ma/mgcp (use internal directly).
- Delete usecases/eg/lpc/validators.go and validators_test.go: the
  three validators only re-checked fields that the upstream filters
  or existing nil guards already guarantee; the redundant Validate()
  calls in eg/lpc/public.go are removed along with them, and an
  explicit CharacteristicId nil guard is added where relevant.
- Remove VALIDATION.md; it documented the now-deleted legacy
  validators and is out of date.

Tests for the affected packages were reshaped around the new,
smaller API surface.
andig
andig reviewed Apr 16, 2026
View reviewed changes
Comment thread usecases/eg/lpc/public.go Outdated
@DerAndereAndi
fix(eg/lpc): guard ConsumptionNominalMax against len != 1
e1d2106 
Match the convention used by MPC Power/EnergyConsumed/Frequency and
MGCP Power, which treat a filter result with more than one match the
same as no match: a protocol violation should surface as
ErrDataNotAvailable rather than silently returning the first entry.

Addresses review comment from @andig on public.go:288.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sthelen-enqs sthelen-enqs sthelen-enqs requested changes

+1 more reviewer

@andig andig andig left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@DerAndereAndi @andig @sthelen-enqs