EU Data Act: store data points by key/name, live SoC fix

[andig]

Fix #31256

Stores delivered data points in a slice keyed by GUID, falling back to the field name, so non-unique data field names can be disambiguated by key.

The official V5.0 data dictionary (added as vehicle/vw/eudataact/datadictionary.json for reference) confirms many field names are not unique: battery_state_report.soc maps to 4 GUIDs, settings.target_soc to 4, mileage.value to 3, charging_state_report.* to 4 each.

SoC now reads the live fields only — battery_level_HV.value, state_of_charge, hv_soc. battery_state_report.soc is dropped: the dictionary defines it as "State of Charging when charging is started" (a charge-start snapshot, not the live value), and the battery_level_HV.state == VALID gating is removed.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• The new find helper does a linear scan over the entire slice on every lookup and is now used in merge, lookup, and tests; if the number of points per vehicle grows this could become a hot path, so consider maintaining a small auxiliary index (e.g. a map[id]int]) or precomputing a map when decoding to avoid repeated O(n) scans.
• Given that point.id() prefers Key over Name while find matches both Key and Name, it might be worth adding a small comment or invariant check to ensure there are no cases where two distinct logical signals share the same Name but differ in Key, since that could make find ambiguous despite the deduplication logic.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The new `find` helper does a linear scan over the entire slice on every lookup and is now used in `merge`, `lookup`, and tests; if the number of points per vehicle grows this could become a hot path, so consider maintaining a small auxiliary index (e.g. a `map[id]int]`) or precomputing a map when decoding to avoid repeated O(n) scans.
- Given that `point.id()` prefers `Key` over `Name` while `find` matches both `Key` and `Name`, it might be worth adding a small comment or invariant check to ensure there are no cases where two distinct logical signals share the same `Name` but differ in `Key`, since that could make `find` ambiguous despite the deduplication logic.

## Individual Comments

### Comment 1
<location path="vehicle/vw/eudataact/types.go" line_range="202-211" />
<code_context>
-		res[name] = p
-	}
+// points decodes data points, keeping the newest entry per id (see point.id).
+func points(data []dataPoint) []point {
+	var res []point

-	for _, p := range data {
-		if p.Value == "" {
+	for _, dp := range data {
+		if dp.Value == "" {
 			continue
 		}

 		var ts time.Time
-		if p.TimestampUtc != nil {
-			ts = *p.TimestampUtc
+		if dp.TimestampUtc != nil {
+			ts = *dp.TimestampUtc
+		}
+		p := point{Key: dp.Key, Name: dp.DataFieldName, Value: dp.Value, Timestamp: ts}
+		if p.id() == "" {
+			continue
 		}
-		pt := point{Key: p.Key, Name: p.DataFieldName, Value: p.Value, Timestamp: ts}

-		set(p.DataFieldName, pt)
-		if _, ok := knownKeys[p.Key]; ok {
-			set(p.Key, pt)
+		if e := find(res, p.id()); e != nil {
+			// newest wins; on equal timestamps the later entry wins
+			if !e.Timestamp.After(p.Timestamp) {
</code_context>
<issue_to_address>
**suggestion (performance):** Current slice-based deduplication is O(n²); consider a map-based index to avoid quadratic behavior on larger datasets.

Using `find` on an ever-growing `[]point` makes deduplication O(n²). If this runs on larger datasets or in a hot path, it may become a bottleneck. Consider maintaining a `map[string]int` (id → index into `res`) while building `res`, so each lookup is O(1) and order is preserved. The same index map can be reused to avoid repeated scans in `merge` and `lookup`.

Suggested implementation:

```golang
 // points decodes data points, keeping the newest entry per id (see point.id).
 // It preserves the order of first occurrence while deduplicating by id.
func points(data []dataPoint) []point {
	var (
		res   []point
		index = make(map[string]int, len(data)) // id -> index in res
	)

	for _, dp := range data {
		if dp.Value == "" {
			continue
		}

		var ts time.Time
		if dp.TimestampUtc != nil {
			ts = *dp.TimestampUtc
		}

		p := point{
			Key:       dp.Key,
			Name:      dp.DataFieldName,
			Value:     dp.Value,
			Timestamp: ts,
		}

		id := p.id()
		if id == "" {
			continue
		}

		if i, ok := index[id]; ok {
			// newest wins; on equal timestamps the later entry wins
			existing := &res[i]
			if !existing.Timestamp.After(p.Timestamp) {
				*existing = p
			}
			continue
		}

		index[id] = len(res)
		res = append(res, p)
	}

	return res
}

```

The comment also suggests reusing an index map in `merge` and `lookup`. To fully implement that:
1. Identify any `merge` / `lookup` helpers that currently scan `[]point` linearly (possibly using `find`).
2. Introduce a shared `map[string]int` (or pass it as a parameter) in those functions, using the same pattern as above to avoid repeated O(n) scans.
3. Remove or limit use of `find` where the map-based index is available, to ensure lookups and merges remain O(1) per operation.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

suggestion (performance): Current slice-based deduplication is O(n²); consider a map-based index to avoid quadratic behavior on larger datasets.

Using find on an ever-growing []point makes deduplication O(n²). If this runs on larger datasets or in a hot path, it may become a bottleneck. Consider maintaining a map[string]int (id → index into res) while building res, so each lookup is O(1) and order is preserved. The same index map can be reused to avoid repeated scans in merge and lookup.

Suggested implementation:

 // points decodes data points, keeping the newest entry per id (see point.id).
 // It preserves the order of first occurrence while deduplicating by id.
func points(data []dataPoint) []point {
	var (
		res   []point
		index = make(map[string]int, len(data)) // id -> index in res
	)

	for _, dp := range data {
		if dp.Value == "" {
			continue
		}

		var ts time.Time
		if dp.TimestampUtc != nil {
			ts = *dp.TimestampUtc
		}

		p := point{
			Key:       dp.Key,
			Name:      dp.DataFieldName,
			Value:     dp.Value,
			Timestamp: ts,
		}

		id := p.id()
		if id == "" {
			continue
		}

		if i, ok := index[id]; ok {
			// newest wins; on equal timestamps the later entry wins
			existing := &res[i]
			if !existing.Timestamp.After(p.Timestamp) {
				*existing = p
			}
			continue
		}

		index[id] = len(res)
		res = append(res, p)
	}

	return res
}

The comment also suggests reusing an index map in merge and lookup. To fully implement that:

1. Identify any merge / lookup helpers that currently scan []point linearly (possibly using find).
2. Introduce a shared map[string]int (or pass it as a parameter) in those functions, using the same pattern as above to avoid repeated O(n) scans.
3. Remove or limit use of find where the map-based index is available, to ensure lookups and merges remain O(1) per operation.