Fix phantom FOVs, reorderable profiles

This commit is contained in:
stopflock
2026-03-02 12:38:49 -06:00
parent b56e9325b3
commit bc671c4efe
7 changed files with 358 additions and 78 deletions

View File

@@ -1,4 +1,10 @@
{
"2.8.1": {
"content": [
"• Fixed bug where the \"existing tags\" profile would incorrectly add default FOV ranges during submission",
"• Added drag handles so profiles can be reordered to customize dropdown order when submitting"
]
},
"2.8.0": {
"content": [
"• Update dependencies and build chain tools; no code changes"

View File

@@ -407,6 +407,10 @@ class AppState extends ChangeNotifier {
_profileState.addOrUpdateProfile(p);
}
void reorderProfiles(int oldIndex, int newIndex) {
_profileState.reorderProfiles(oldIndex, newIndex);
}
void deleteProfile(NodeProfile p) {
_profileState.deleteProfile(p);
}

View File

@@ -269,16 +269,33 @@ class NodeProfile {
/// Used as the default `<Existing tags>` option when editing nodes
/// All existing tags will flow through as additionalExistingTags
static NodeProfile createExistingTagsProfile(OsmNode node) {
// Calculate FOV from existing direction ranges if applicable
// Only assign FOV if the original direction string actually contained range notation
// (e.g., "90-270" or "55-125"), not if it was just single directions (e.g., "90")
double? calculatedFov;
// If node has direction/FOV pairs, check if they all have the same FOV
if (node.directionFovPairs.isNotEmpty) {
final firstFov = node.directionFovPairs.first.fovDegrees;
final raw = node.tags['direction'] ?? node.tags['camera:direction'];
if (raw != null) {
// Check if any part of the direction string contains range notation (dash with numbers)
final parts = raw.split(';');
bool hasRangeNotation = false;
// If all directions have the same FOV, use it for the profile
if (node.directionFovPairs.every((df) => df.fovDegrees == firstFov)) {
calculatedFov = firstFov;
for (final part in parts) {
final trimmed = part.trim();
// Look for range pattern: numbers-numbers (e.g., "90-270", "55-125")
if (trimmed.contains('-') && RegExp(r'^\d+\.?\d*-\d+\.?\d*$').hasMatch(trimmed)) {
hasRangeNotation = true;
break;
}
}
// Only calculate FOV if the node originally had range notation
if (hasRangeNotation && node.directionFovPairs.isNotEmpty) {
final firstFov = node.directionFovPairs.first.fovDegrees;
// If all directions have the same FOV, use it for the profile
if (node.directionFovPairs.every((df) => df.fovDegrees == firstFov)) {
calculatedFov = firstFov;
}
}
}
@@ -290,7 +307,7 @@ class NodeProfile {
requiresDirection: true,
submittable: true,
editable: false,
fov: calculatedFov, // Use calculated FOV from existing direction ranges
fov: calculatedFov, // Only use FOV if original had explicit range notation
);
}

View File

@@ -34,76 +34,101 @@ class NodeProfilesSection extends StatelessWidget {
),
],
),
...appState.profiles.map(
(p) => ListTile(
leading: Checkbox(
value: appState.isEnabled(p),
onChanged: (v) => appState.toggleProfile(p, v ?? false),
),
title: Text(p.name),
subtitle: Text(p.builtin ? locService.t('profiles.builtIn') : locService.t('profiles.custom')),
trailing: !p.editable
? PopupMenuButton(
itemBuilder: (context) => [
PopupMenuItem(
value: 'view',
child: Row(
children: [
const Icon(Icons.visibility),
const SizedBox(width: 8),
Text(locService.t('profiles.view')),
],
),
ReorderableListView.builder(
shrinkWrap: true,
physics: const NeverScrollableScrollPhysics(),
itemCount: appState.profiles.length,
onReorder: (oldIndex, newIndex) {
appState.reorderProfiles(oldIndex, newIndex);
},
itemBuilder: (context, index) {
final p = appState.profiles[index];
return ListTile(
key: ValueKey(p.id),
leading: Row(
mainAxisSize: MainAxisSize.min,
children: [
// Drag handle
ReorderableDragStartListener(
index: index,
child: const Icon(
Icons.drag_handle,
color: Colors.grey,
),
],
onSelected: (value) {
if (value == 'view') {
Navigator.push(
context,
MaterialPageRoute(
builder: (_) => ProfileEditor(profile: p),
),
const SizedBox(width: 8),
// Checkbox
Checkbox(
value: appState.isEnabled(p),
onChanged: (v) => appState.toggleProfile(p, v ?? false),
),
],
),
title: Text(p.name),
subtitle: Text(p.builtin ? locService.t('profiles.builtIn') : locService.t('profiles.custom')),
trailing: !p.editable
? PopupMenuButton(
itemBuilder: (context) => [
PopupMenuItem(
value: 'view',
child: Row(
children: [
const Icon(Icons.visibility),
const SizedBox(width: 8),
Text(locService.t('profiles.view')),
],
),
);
}
},
)
: PopupMenuButton(
itemBuilder: (context) => [
PopupMenuItem(
value: 'edit',
child: Row(
children: [
const Icon(Icons.edit),
const SizedBox(width: 8),
Text(locService.t('actions.edit')),
],
),
),
PopupMenuItem(
value: 'delete',
child: Row(
children: [
const Icon(Icons.delete, color: Colors.red),
const SizedBox(width: 8),
Text(locService.t('profiles.deleteProfile'), style: const TextStyle(color: Colors.red)),
],
),
),
],
onSelected: (value) {
if (value == 'edit') {
Navigator.push(
context,
MaterialPageRoute(
builder: (_) => ProfileEditor(profile: p),
],
onSelected: (value) {
if (value == 'view') {
Navigator.push(
context,
MaterialPageRoute(
builder: (_) => ProfileEditor(profile: p),
),
);
}
},
)
: PopupMenuButton(
itemBuilder: (context) => [
PopupMenuItem(
value: 'edit',
child: Row(
children: [
const Icon(Icons.edit),
const SizedBox(width: 8),
Text(locService.t('actions.edit')),
],
),
);
} else if (value == 'delete') {
_showDeleteProfileDialog(context, p);
}
},
),
),
),
PopupMenuItem(
value: 'delete',
child: Row(
children: [
const Icon(Icons.delete, color: Colors.red),
const SizedBox(width: 8),
Text(locService.t('profiles.deleteProfile'), style: const TextStyle(color: Colors.red)),
],
),
),
],
onSelected: (value) {
if (value == 'edit') {
Navigator.push(
context,
MaterialPageRoute(
builder: (_) => ProfileEditor(profile: p),
),
);
} else if (value == 'delete') {
_showDeleteProfileDialog(context, p);
}
},
),
);
},
),
],
);

View File

@@ -6,9 +6,11 @@ import '../services/profile_service.dart';
class ProfileState extends ChangeNotifier {
static const String _enabledPrefsKey = 'enabled_profiles';
static const String _profileOrderPrefsKey = 'profile_order';
final List<NodeProfile> _profiles = [];
final Set<NodeProfile> _enabled = {};
List<String> _customOrder = []; // List of profile IDs in user's preferred order
// Callback for when a profile is deleted (used to clear stale sessions)
void Function(NodeProfile)? _onProfileDeleted;
@@ -18,10 +20,10 @@ class ProfileState extends ChangeNotifier {
}
// Getters
List<NodeProfile> get profiles => List.unmodifiable(_profiles);
List<NodeProfile> get profiles => List.unmodifiable(_getOrderedProfiles());
bool isEnabled(NodeProfile p) => _enabled.contains(p);
List<NodeProfile> get enabledProfiles =>
_profiles.where(isEnabled).toList(growable: false);
_getOrderedProfiles().where(isEnabled).toList(growable: false);
// Initialize profiles from built-in and custom sources
Future<void> init({bool addDefaults = false}) async {
@@ -34,7 +36,7 @@ class ProfileState extends ChangeNotifier {
await ProfileService().save(_profiles);
}
// Load enabled profile IDs from prefs
// Load enabled profile IDs and custom order from prefs
final prefs = await SharedPreferences.getInstance();
final enabledIds = prefs.getStringList(_enabledPrefsKey);
if (enabledIds != null && enabledIds.isNotEmpty) {
@@ -44,6 +46,9 @@ class ProfileState extends ChangeNotifier {
// By default, all are enabled
_enabled.addAll(_profiles);
}
// Load custom order
_customOrder = prefs.getStringList(_profileOrderPrefsKey) ?? [];
}
void toggleProfile(NodeProfile p, bool e) {
@@ -92,6 +97,45 @@ class ProfileState extends ChangeNotifier {
notifyListeners();
}
// Reorder profiles (for drag-and-drop in settings)
void reorderProfiles(int oldIndex, int newIndex) {
final orderedProfiles = _getOrderedProfiles();
if (oldIndex < newIndex) {
newIndex -= 1;
}
final item = orderedProfiles.removeAt(oldIndex);
orderedProfiles.insert(newIndex, item);
// Update custom order with new sequence
_customOrder = orderedProfiles.map((p) => p.id).toList();
_saveCustomOrder();
notifyListeners();
}
// Get profiles in custom order, with unordered profiles at the end
List<NodeProfile> _getOrderedProfiles() {
if (_customOrder.isEmpty) {
return List.from(_profiles);
}
final ordered = <NodeProfile>[];
final profilesById = {for (final p in _profiles) p.id: p};
// Add profiles in custom order
for (final id in _customOrder) {
final profile = profilesById[id];
if (profile != null) {
ordered.add(profile);
profilesById.remove(id);
}
}
// Add any remaining profiles that weren't in the custom order
ordered.addAll(profilesById.values);
return ordered;
}
// Save enabled profile IDs to disk
Future<void> _saveEnabledProfiles() async {
final prefs = await SharedPreferences.getInstance();
@@ -100,4 +144,10 @@ class ProfileState extends ChangeNotifier {
_enabled.map((p) => p.id).toList(),
);
}
// Save custom order to disk
Future<void> _saveCustomOrder() async {
final prefs = await SharedPreferences.getInstance();
await prefs.setStringList(_profileOrderPrefsKey, _customOrder);
}
}

View File

@@ -1,7 +1,7 @@
name: deflockapp
description: Map public surveillance infrastructure with OpenStreetMap
publish_to: "none"
version: 2.8.0+49 # The thing after the + is the version code, incremented with each release
version: 2.8.1+50 # The thing after the + is the version code, incremented with each release
environment:
sdk: ">=3.10.3 <4.0.0" # Resolved dependency floor (Dart 3.10.3 = Flutter 3.38+)

View File

@@ -1,5 +1,8 @@
import 'package:flutter_test/flutter_test.dart';
import 'package:latlong2/latlong.dart';
import 'package:deflockapp/models/node_profile.dart';
import 'package:deflockapp/models/osm_node.dart';
import 'package:deflockapp/state/profile_state.dart';
void main() {
group('NodeProfile', () {
@@ -72,5 +75,180 @@ void main() {
expect(a.hashCode, equals(b.hashCode));
expect(a, isNot(equals(c)));
});
group('createExistingTagsProfile', () {
test('should NOT assign FOV for nodes with single direction', () {
// This is the core bug fix: nodes with just "direction=90" should not get a default FOV
final node = OsmNode(
id: 123,
coord: const LatLng(37.7749, -122.4194),
tags: {
'direction': '90',
'man_made': 'surveillance',
'surveillance:type': 'ALPR',
},
);
final profile = NodeProfile.createExistingTagsProfile(node);
expect(profile.fov, isNull, reason: 'Single direction nodes should not get default FOV');
expect(profile.name, equals('<Existing tags>'));
expect(profile.tags, isEmpty, reason: 'Existing tags profile should have empty tags');
});
test('should assign FOV for nodes with range notation', () {
final node = OsmNode(
id: 123,
coord: const LatLng(37.7749, -122.4194),
tags: {
'direction': '55-125', // Range notation = explicit FOV
'man_made': 'surveillance',
'surveillance:type': 'ALPR',
},
);
final profile = NodeProfile.createExistingTagsProfile(node);
expect(profile.fov, isNotNull, reason: 'Range notation should preserve FOV');
expect(profile.fov, equals(70.0), reason: 'Range 55-125 should calculate to 70 degree FOV');
});
test('should assign FOV for nodes with multiple consistent ranges', () {
final node = OsmNode(
id: 123,
coord: const LatLng(37.7749, -122.4194),
tags: {
'direction': '55-125;235-305', // Two ranges with same FOV
'man_made': 'surveillance',
'surveillance:type': 'ALPR',
},
);
final profile = NodeProfile.createExistingTagsProfile(node);
expect(profile.fov, equals(70.0), reason: 'Multiple consistent ranges should preserve FOV');
});
test('should NOT assign FOV for mixed single directions and ranges', () {
final node = OsmNode(
id: 123,
coord: const LatLng(37.7749, -122.4194),
tags: {
'direction': '90;180-360', // Mix of single direction and range
'man_made': 'surveillance',
'surveillance:type': 'ALPR',
},
);
final profile = NodeProfile.createExistingTagsProfile(node);
expect(profile.fov, isNull, reason: 'Mixed notation should not assign FOV');
});
test('should NOT assign FOV for multiple single directions', () {
final node = OsmNode(
id: 123,
coord: const LatLng(37.7749, -122.4194),
tags: {
'direction': '90;180;270', // Multiple single directions
'man_made': 'surveillance',
'surveillance:type': 'ALPR',
},
);
final profile = NodeProfile.createExistingTagsProfile(node);
expect(profile.fov, isNull, reason: 'Multiple single directions should not get default FOV');
});
test('should handle camera:direction tag', () {
final node = OsmNode(
id: 123,
coord: const LatLng(37.7749, -122.4194),
tags: {
'camera:direction': '180', // Using camera:direction instead of direction
'man_made': 'surveillance',
'surveillance:type': 'camera',
},
);
final profile = NodeProfile.createExistingTagsProfile(node);
expect(profile.fov, isNull, reason: 'Single camera:direction should not get default FOV');
});
test('should fix the specific bug: direction=90 should not become direction=55-125', () {
// This tests the exact bug scenario mentioned in the issue
final node = OsmNode(
id: 123,
coord: const LatLng(37.7749, -122.4194),
tags: {
'direction': '90', // Single direction, should stay as single direction
'man_made': 'surveillance',
'surveillance:type': 'ALPR',
},
);
final profile = NodeProfile.createExistingTagsProfile(node);
// Key fix: profile should NOT have an FOV, so upload won't convert to range notation
expect(profile.fov, isNull, reason: 'direction=90 should not get converted to direction=55-125');
// Verify the node does have directionFovPairs (for rendering), but profile ignores them
expect(node.directionFovPairs, hasLength(1));
expect(node.directionFovPairs.first.centerDegrees, equals(90.0));
expect(node.directionFovPairs.first.fovDegrees, equals(70.0)); // Default FOV for rendering
});
});
group('ProfileState reordering', () {
test('should reorder profiles correctly', () {
final profileState = ProfileState();
// Add some test profiles
final profileA = NodeProfile(id: 'a', name: 'Profile A', tags: const {});
final profileB = NodeProfile(id: 'b', name: 'Profile B', tags: const {});
final profileC = NodeProfile(id: 'c', name: 'Profile C', tags: const {});
profileState.addOrUpdateProfile(profileA);
profileState.addOrUpdateProfile(profileB);
profileState.addOrUpdateProfile(profileC);
// Initial order should be A, B, C
expect(profileState.profiles.map((p) => p.id), equals(['a', 'b', 'c']));
// Move profile at index 0 (A) to index 2 (should become B, C, A)
profileState.reorderProfiles(0, 2);
expect(profileState.profiles.map((p) => p.id), equals(['b', 'c', 'a']));
// Move profile at index 2 (A) to index 1 (should become B, A, C)
profileState.reorderProfiles(2, 1);
expect(profileState.profiles.map((p) => p.id), equals(['b', 'a', 'c']));
});
test('should maintain enabled status after reordering', () {
final profileState = ProfileState();
final profileA = NodeProfile(id: 'a', name: 'Profile A', tags: const {});
final profileB = NodeProfile(id: 'b', name: 'Profile B', tags: const {});
final profileC = NodeProfile(id: 'c', name: 'Profile C', tags: const {});
profileState.addOrUpdateProfile(profileA);
profileState.addOrUpdateProfile(profileB);
profileState.addOrUpdateProfile(profileC);
// Disable profile B
profileState.toggleProfile(profileB, false);
expect(profileState.isEnabled(profileB), isFalse);
// Reorder profiles
profileState.reorderProfiles(0, 2);
// Profile B should still be disabled after reordering
expect(profileState.isEnabled(profileB), isFalse);
expect(profileState.isEnabled(profileA), isTrue);
expect(profileState.isEnabled(profileC), isTrue);
});
});
});
}