Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Settings ui and service #717

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Settings ui and service #717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac4583d
Settings ui and service
DTTerastar Sep 9, 2024
b67f6f1
Fix C# Code
DTTerastar Sep 10, 2024
b25f703
WIP
DTTerastar Sep 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions Controllers/SettingsController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
using Microsoft.AspNetCore.Mvc;
using System.IO;
using System.Text.Json;

[ApiController]
[Route("api/settings")]
public class SettingsController : ControllerBase
{
private const string SettingsFilePath = "settings.json";

Comment on lines +5 to +10
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider making the settings file path configurable.

The SettingsFilePath is currently hardcoded as a constant. To improve flexibility across different environments, consider making this path configurable through application settings or environment variables.

You could modify the controller to accept the file path through dependency injection:

public class SettingsController : ControllerBase
{
    private readonly string _settingsFilePath;

    public SettingsController(IConfiguration configuration)
    {
        _settingsFilePath = configuration["SettingsFilePath"] ?? "settings.json";
    }

    // ... rest of the controller
}

[HttpGet]
public ActionResult<Settings> GetSettings()
{
if (!System.IO.File.Exists(SettingsFilePath))
{
return new Settings();
}

var json = System.IO.File.ReadAllText(SettingsFilePath);
return JsonSerializer.Deserialize<Settings>(json);
}

[HttpPost]
public IActionResult SaveSettings([FromBody] Settings settings)
{
var json = JsonSerializer.Serialize(settings);
System.IO.File.WriteAllText(SettingsFilePath, json);
return Ok();
}
}

public class Settings
{
public Updating Updating { get; set; }
public Scanning Scanning { get; set; }
public Counting Counting { get; set; }
public Filtering Filtering { get; set; }
public Calibration Calibration { get; set; }
}
Comment on lines +32 to +39
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Initialize properties to prevent null reference exceptions.

The Settings class properties are of reference types but are not initialized. This could lead to null reference exceptions if these properties are accessed before being set.

Consider initializing the properties in the constructor:

public class Settings
{
    public Settings()
    {
        Updating = new Updating();
        Scanning = new Scanning();
        Counting = new Counting();
        Filtering = new Filtering();
        Calibration = new Calibration();
    }

    public Updating Updating { get; set; }
    public Scanning Scanning { get; set; }
    public Counting Counting { get; set; }
    public Filtering Filtering { get; set; }
    public Calibration Calibration { get; set; }
}


public class Updating
{
public bool AutoUpdate { get; set; }
public bool PreRelease { get; set; }
}

public class Scanning
{
public int? ForgetAfterMs { get; set; }
}

public class Counting
{
public string IdPrefixes { get; set; }
public double? StartCountingDistance { get; set; }
public double? StopCountingDistance { get; set; }
public int? IncludeDevicesAge { get; set; }
}

public class Filtering
{
public string IncludeIds { get; set; }
public string ExcludeIds { get; set; }
public double? MaxReportDistance { get; set; }
public double? EarlyReportDistance { get; set; }
public int? SkipReportAge { get; set; }
}

public class Calibration
{
public int? RssiAt1m { get; set; }
public int? RssiAdjustment { get; set; }
public double? AbsorptionFactor { get; set; }
public int? IBeaconRssiAt1m { get; set; }
}
24 changes: 14 additions & 10 deletions src/Controllers/DeviceController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,26 @@
_state = state;
}

[HttpGet("{id}")]
public DeviceSettingsDetails Get(string id)
[HttpGet("{id}/details")]
public async Task<IList<KeyValuePair<string, string>>> Details(string id)

Check warning on line 23 in src/Controllers/DeviceController.cs

View workflow job for this annotation

GitHub Actions / build 

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
{
var deviceSettings = _deviceSettingsStore.Get(id);
var details = new List<KeyValuePair<string, string>>();
if (deviceSettings?.Id != null && _state.Devices.TryGetValue(deviceSettings.Id, out var device))
details.AddRange(device.GetDetails());
return new DeviceSettingsDetails(deviceSettings ?? new DeviceSettings { Id = id, OriginalId = id }, details);
if (_state.Devices.TryGetValue(id, out var device))
return device.GetDetails().ToList();
return new List<KeyValuePair<string, string>>();
}

[HttpPut("{id}")]
[HttpGet("{id}/settings")]
public Task<DeviceSettings?> Get(string id)
{
var settings = _deviceSettingsStore.Get(id);
settings ??= new DeviceSettings { OriginalId = id, Id = id };
return Task.FromResult(settings);

Check warning on line 35 in src/Controllers/DeviceController.cs

View workflow job for this annotation

GitHub Actions / build 

Nullability of reference types in value of type 'Task<DeviceSettings>' doesn't match target type 'Task<DeviceSettings?>'.
}
Comment on lines +30 to +36
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Address nullability mismatch and consider optimization

The Get method has been updated to return a Task<DeviceSettings?>, but there's a nullability mismatch with the actual returned value.

  1. To address the nullability warning, update the return type:
-        public Task<DeviceSettings?> Get(string id)
+        public Task<DeviceSettings> Get(string id)
  1. Consider optimizing the method by using C# 8.0's null-coalescing assignment operator:
-            var settings = _deviceSettingsStore.Get(id);
-            settings ??= new DeviceSettings { OriginalId = id, Id = id };
+            var settings = _deviceSettingsStore.Get(id) ?? new DeviceSettings { OriginalId = id, Id = id };

These changes will resolve the nullability warning and slightly improve the code's conciseness.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[HttpGet("{id}/settings")]
public Task<DeviceSettings?> Get(string id)
{
var settings = _deviceSettingsStore.Get(id);
settings ??= new DeviceSettings { OriginalId = id, Id = id };
return Task.FromResult(settings);
}
[HttpGet("{id}/settings")]
public Task<DeviceSettings> Get(string id)
{
var settings = _deviceSettingsStore.Get(id) ?? new DeviceSettings { OriginalId = id, Id = id };
return Task.FromResult(settings);
}
🧰 Tools
🪛 GitHub Check: build

[warning] 35-35:
Nullability of reference types in value of type 'Task' doesn't match target type 'Task<DeviceSettings?>'.


[HttpPut("{id}/settings")]
public async Task Set(string id, [FromBody] DeviceSettings value)
{
await _deviceSettingsStore.Set(id, value);
}
}

public readonly record struct DeviceSettingsDetails(DeviceSettings? settings, IList<KeyValuePair<string, string>> details);
}
10 changes: 5 additions & 5 deletions src/Controllers/NodeController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,18 @@ namespace ESPresense.Controllers;
[ApiController]
public class NodeController(NodeSettingsStore nodeSettingsStore, State state) : ControllerBase
{
[HttpGet("{id}")]
[HttpGet("{id}/settings")]
public NodeSettingsDetails Get(string id)
{
var nodeSettings = nodeSettingsStore.Get(id);
var details = new List<KeyValuePair<string, string>>();
if (nodeSettings?.Id != null && state.Nodes.TryGetValue(id, out var node))
details.AddRange(node.GetDetails());
return new NodeSettingsDetails(nodeSettings ?? new NodeSettings(id), details);
return new NodeSettingsDetails(nodeSettings ?? new Models.NodeSettings(id), details);
}

[HttpPut("{id}")]
public Task Set(string id, [FromBody] NodeSettings ds)
[HttpPut("{id}/settings")]
public Task Set(string id, [FromBody] Models.NodeSettings ds)
{
Log.Information("Set {id} {@ds}", id, ds);
return nodeSettingsStore.Set(id, ds);
Expand All @@ -38,5 +38,5 @@ public async Task Restart(string id)
await nodeSettingsStore.Restart(id);
}

public readonly record struct NodeSettingsDetails(NodeSettings? settings, IList<KeyValuePair<string, string>> details);
public readonly record struct NodeSettingsDetails(Models.NodeSettings? settings, IList<KeyValuePair<string, string>> details);
}
6 changes: 3 additions & 3 deletions src/Services/NodeSettingsStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,28 @@
}
private readonly ConcurrentDictionary<string, NodeSettings> _storeById = new();

public NodeSettings Get(string id)
public Models.NodeSettings Get(string id)
{
return _storeById.TryGetValue(id, out var ns) ? ns.Clone() : new NodeSettings(id);
return _storeById.TryGetValue(id, out var ns) ? ns.Clone() : new Models.NodeSettings(id);
Comment on lines +24 to +26
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure Calibration is not null to prevent NullReferenceException.

In the Get method, when creating a new Models.NodeSettings, verify that the Calibration property is initialized. If Calibration is null, accessing it later will cause a NullReferenceException.

Apply this diff to initialize Calibration:

-return _storeById.TryGetValue(id, out var ns) ? ns.Clone() : new Models.NodeSettings(id);
+return _storeById.TryGetValue(id, out var ns) ? ns.Clone() : new Models.NodeSettings(id)
+{
+    Calibration = new CalibrationSettings(),
+    Filtering = new FilteringSettings()
+};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public Models.NodeSettings Get(string id)
{
return _storeById.TryGetValue(id, out var ns) ? ns.Clone() : new NodeSettings(id);
return _storeById.TryGetValue(id, out var ns) ? ns.Clone() : new Models.NodeSettings(id);
public Models.NodeSettings Get(string id)
{
return _storeById.TryGetValue(id, out var ns) ? ns.Clone() : new Models.NodeSettings(id)
{
Calibration = new CalibrationSettings(),
Filtering = new FilteringSettings()
};
}

}

public async Task Set(string id, NodeSettings ds)
public async Task Set(string id, Models.NodeSettings ns)
{
var retain = id == "*";
var old = Get(id);

// Updating settings
if (ds.Updating.AutoUpdate == null || ds.Updating.AutoUpdate != old.Updating.AutoUpdate)

Check failure on line 35 in src/Services/NodeSettingsStore.cs

View workflow job for this annotation

GitHub Actions / build 

The name 'ds' does not exist in the current context

Check failure on line 35 in src/Services/NodeSettingsStore.cs

View workflow job for this annotation

GitHub Actions / build 

The name 'ds' does not exist in the current context
await mqtt.EnqueueAsync($"espresense/rooms/{id}/auto_update/set", ds.Updating.AutoUpdate == true ? "ON" : "OFF", retain);

Check failure on line 36 in src/Services/NodeSettingsStore.cs

View workflow job for this annotation

GitHub Actions / build 

The name 'ds' does not exist in the current context
if (ds.Updating.Prerelease == null || ds.Updating.Prerelease != old.Updating.Prerelease)

Check failure on line 37 in src/Services/NodeSettingsStore.cs

View workflow job for this annotation

GitHub Actions / build 

The name 'ds' does not exist in the current context

Check failure on line 37 in src/Services/NodeSettingsStore.cs

View workflow job for this annotation

GitHub Actions / build 

The name 'ds' does not exist in the current context
await mqtt.EnqueueAsync($"espresense/rooms/{id}/prerelease/set", ds.Updating.Prerelease == true ? "ON" : "OFF", retain);

Check failure on line 38 in src/Services/NodeSettingsStore.cs

View workflow job for this annotation

GitHub Actions / build 

The name 'ds' does not exist in the current context

// Scanning settings
if (ds.Scanning.ForgetAfterMs == null || ds.Scanning.ForgetAfterMs != old.Scanning.ForgetAfterMs)

Check failure on line 41 in src/Services/NodeSettingsStore.cs

View workflow job for this annotation

GitHub Actions / build 

The name 'ds' does not exist in the current context

Check failure on line 41 in src/Services/NodeSettingsStore.cs

View workflow job for this annotation

GitHub Actions / build 

The name 'ds' does not exist in the current context
await mqtt.EnqueueAsync($"espresense/rooms/{id}/forget_after_ms/set", $"{ds.Scanning.ForgetAfterMs}", retain);

Check failure on line 42 in src/Services/NodeSettingsStore.cs

View workflow job for this annotation

GitHub Actions / build 

The name 'ds' does not exist in the current context

// Counting settings
if (ds.Counting.IdPrefixes == null || ds.Counting.IdPrefixes != old.Counting.IdPrefixes)

Check failure on line 45 in src/Services/NodeSettingsStore.cs

View workflow job for this annotation

GitHub Actions / build 

The name 'ds' does not exist in the current context
await mqtt.EnqueueAsync($"espresense/rooms/{id}/count_ids/set", $"{ds.Counting.IdPrefixes}", retain);
if (ds.Counting.MinDistance == null || ds.Counting.MinDistance != old.Counting.MinDistance)
await mqtt.EnqueueAsync($"espresense/rooms/{id}/count_min_dist/set", $"{ds.Counting.MinDistance:0.00}", retain);
Expand Down
155 changes: 155 additions & 0 deletions src/ui/src/lib/GlobalSettings.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
<script lang="ts">
import TriStateCheckbox from '$lib/TriStateCheckbox.svelte';
import { settings } from '$lib/stores';
import { onMount } from 'svelte';

let loading = true;
let error: string | null = null;

onMount(async () => {
try {
await settings.load();
} catch (e: unknown) {
error = e instanceof Error ? e.message : 'An unknown error occurred';
} finally {
loading = false;
}
});

async function handleUpdate() {
try {
if ($settings) {
await settings.save($settings);
}
} catch (e: unknown) {
error = e instanceof Error ? `Error updating settings: ${e.message}` : 'An unknown error occurred while updating settings';
}
}
</script>

{#if loading}
<div class="card m-2 p-4 variant-filled-surface">
<div class="flex items-center space-x-4">
<span class="loading loading-spinner loading-lg" />
<p>Loading settings...</p>
</div>
</div>
{:else if error}
<div class="card m-2 p-4 variant-filled-error">
<p>Error: {error}</p>
</div>
{:else if $settings}
<div class="card m-2 p-4 variant-filled-surface">
<section class="mb-8">
<h2 class="text-2xl font-semibold mb-4">Updating</h2>
<div class="space-y-4 ml-4">
<div class="flex items-center space-x-2">
<TriStateCheckbox id="auto-update" bind:checked={$settings.updating.autoUpdate} />
<label for="auto-update">Automatically update</label>
</div>
<div class="flex items-center space-x-2">
<TriStateCheckbox id="pre-release" bind:checked={$settings.updating.preRelease} />
<label for="pre-release">Include pre-released versions in auto-update</label>
</div>
</div>
</section>

<section class="mb-8">
<h2 class="text-2xl font-semibold mb-4">Scanning</h2>
<div class="space-y-4 ml-4">
<label class="label">
<span>Forget beacon if not seen for (in milliseconds):</span>
<input type="number" class="input" min="0" bind:value={$settings.scanning.forgetAfterMs} placeholder="150000" />
</label>
</div>
</section>

<section class="mb-8">
<h2 class="text-2xl font-semibold mb-4">Counting</h2>
<div class="space-y-4 ml-4">
<label class="label">
<span>Include id prefixes (space separated):</span>
<input type="text" class="input" bind:value={$settings.counting.idPrefixes} placeholder="" />
</label>

<label class="label">
<span>Start counting devices less than distance (in meters):</span>
<input type="number" class="input" step="0.01" min="0" bind:value={$settings.counting.startCountingDistance} placeholder="2.00" />
</label>

<label class="label">
<span>Stop counting devices greater than distance (in meters):</span>
<input type="number" class="input" step="0.01" min="0" bind:value={$settings.counting.stopCountingDistance} placeholder="4.00" />
</label>

<label class="label">
<span>Include devices with age less than (in ms):</span>
<input type="number" class="input" min="0" bind:value={$settings.counting.includeDevicesAge} placeholder="30000" />
</label>
</div>
</section>

<section class="mb-8">
<h2 class="text-2xl font-semibold mb-4">Filtering</h2>
<div class="space-y-4 ml-4">
<label class="label">
<span>Include only sending these ids to mqtt (eg. apple:iphone10-6 apple:iphone13-2):</span>
<input type="text" class="input" bind:value={$settings.filtering.includeIds} placeholder="" />
</label>

<label class="label">
<span>Exclude sending these ids to mqtt (eg. exp:20 apple:iphone10-6):</span>
<input type="text" class="input" bind:value={$settings.filtering.excludeIds} placeholder="" />
</label>

<label class="label">
<span>Max report distance (in meters):</span>
<input type="number" class="input" step="0.01" min="0" bind:value={$settings.filtering.maxReportDistance} placeholder="16.00" />
</label>

<label class="label">
<span>Report early if beacon has moved more than this distance (in meters):</span>
<input type="number" class="input" step="0.01" min="0" bind:value={$settings.filtering.earlyReportDistance} placeholder="0.50" />
</label>

<label class="label">
<span>Skip reporting if message age is less that this (in milliseconds):</span>
<input type="number" class="input" min="0" bind:value={$settings.filtering.skipReportAge} placeholder="5000" />
</label>
</div>
</section>

<section class="mb-8">
<h2 class="text-2xl font-semibold mb-4">Calibration</h2>
<div class="space-y-4 ml-4">
<label class="label">
<span>Rssi expected from a 0dBm transmitter at 1 meter (NOT used for iBeacons or Eddystone):</span>
<input type="number" class="input" bind:value={$settings.calibration.rssiAt1m} placeholder="-65" />
</label>

<label class="label">
<span>Rssi adjustment for receiver (use only if you know this device has a weak antenna):</span>
<input type="number" class="input" bind:value={$settings.calibration.rssiAdjustment} placeholder="0" />
</label>

<label class="label">
<span>Factor used to account for absorption, reflection, or diffraction:</span>
<input type="number" class="input" step="0.01" min="0" bind:value={$settings.calibration.absorptionFactor} placeholder="3.50" />
</label>

<label class="label">
<span>Rssi expected from this tx power at 1m (used for node iBeacon):</span>
<input type="number" class="input" bind:value={$settings.calibration.iBeaconRssiAt1m} placeholder="-59" />
</label>
</div>
</section>

<div class="flex justify-end mt-8">
<button class="btn variant-filled-primary" on:click={handleUpdate}>Update Settings</button>
</div>
</div>
{:else}
<div class="card p-4 variant-filled-warning">
<p>No settings available.</p>
</div>
{/if}
14 changes: 5 additions & 9 deletions src/ui/src/lib/NodeActions.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,18 @@
import { getModalStore, getToastStore, type ToastSettings } from '@skeletonlabs/skeleton';
import { updateMethod, flavor, version, artifact, flavorNames } from '$lib/firmware';
import Firmware from '$lib/modals/Firmware.svelte';
import { restartNode, updateNodeSelf } from '$lib/node';
const modalStore = getModalStore();
const toastStore = getToastStore();
async function onRestart(i: Node) {
try {
var response = await fetch(`${base}/api/node/${i.id}/restart`, { method: 'POST' });
if (response.status != 200) throw new Error(response.statusText);
await restartNode(i.id);
toastStore.trigger({ message: i.name + ' asked to reboot', background: 'variant-filled-primary' });
} catch (e) {
console.log(e);
toastStore.trigger({ message: e, background: 'variant-filled-error' });
toastStore.trigger({ message: e instanceof Error ? e.message : String(e), background: 'variant-filled-error' });
}
}
Expand Down Expand Up @@ -44,12 +44,8 @@
});
} else {
if (i) {
fetch(`${base}/api/node/${i.id}/update`, {
method: 'POST',
body: ''
})
.then((response) => {
if (response.status != 200) throw new Error(response.statusText);
updateNodeSelf(i.id)
.then(() => {
const t: ToastSettings = { message: (i.name ?? i.id) + ' asked to update itself', background: 'variant-filled-primary' };
toastStore.trigger(t);
})
Expand Down
Loading
Loading