From 7afc560634ba406b8fd78a756bda26e1a976a5e3 Mon Sep 17 00:00:00 2001 From: Adrian Stevens Date: Wed, 17 Jan 2024 16:50:41 -0800 Subject: [PATCH] Warnings and defensive coding --- Source/v2/Meadow.CLI.Core/DFU/DfuSharp.cs | 9 +++---- .../PackageManager.AssemblyManager.cs | 6 ++--- .../PackageManager.BuildOptions.cs | 2 +- Source/v2/Meadow.Cli/AppManager.cs | 12 ++++----- .../Commands/Current/App/AppDeployCommand.cs | 2 +- .../Commands/Current/App/AppRunCommand.cs | 2 +- .../Command/CloudCommandPublishCommand.cs | 2 +- .../Cloud/Package/CloudPackageListCommand.cs | 2 +- .../Current/Device/DeviceProvisionCommand.cs | 2 +- .../Firmware/FirmwareDefaultCommand.cs | 2 +- .../Current/Firmware/FirmwareWriteCommand.cs | 11 +++++--- .../Current/Port/PortSelectCommand.cs | 5 +++- .../Commands/Legacy/FlashOsCommand.cs | 27 +++++++++++++------ .../v2/Meadow.Cloud.Client/Messages/User.cs | 2 +- .../SerialConnectionTests.cs | 2 +- .../Connections/SerialConnection.cs | 10 +++---- .../Meadow.Hcom/Connections/TcpConnection.cs | 4 +-- .../SerialRequests/RequestBuilder.cs | 2 +- .../DownloadFileStream.cs | 2 +- .../F7FirmwarePackageCollection.cs | 6 ++--- .../Meadow.SoftwareManager/FirmwarePackage.cs | 2 +- 21 files changed, 66 insertions(+), 48 deletions(-) diff --git a/Source/v2/Meadow.CLI.Core/DFU/DfuSharp.cs b/Source/v2/Meadow.CLI.Core/DFU/DfuSharp.cs index e88d813a..13faa55f 100644 --- a/Source/v2/Meadow.CLI.Core/DFU/DfuSharp.cs +++ b/Source/v2/Meadow.CLI.Core/DFU/DfuSharp.cs @@ -278,8 +278,8 @@ public class DfuDevice : IDisposable public DfuDevice(IntPtr device, InterfaceDescriptor interface_descriptor, DfuFunctionDescriptor dfu_descriptor) { - interface_descriptor = interface_descriptor; - dfu_descriptor = dfu_descriptor; + this.interface_descriptor = interface_descriptor; + this.dfu_descriptor = dfu_descriptor; if (NativeMethods.libusb_open(device, ref handle) < 0) { @@ -287,12 +287,11 @@ public DfuDevice(IntPtr device, InterfaceDescriptor interface_descriptor, DfuFun } } - public event UploadingEventHandler Uploading; + public event UploadingEventHandler Uploading = default!; protected virtual void OnUploading(UploadingEventArgs e) { - if (Uploading != null) - Uploading(this, e); + Uploading?.Invoke(this, e); } public void ClaimInterface() { diff --git a/Source/v2/Meadow.CLI.Core/PackageManager/PackageManager.AssemblyManager.cs b/Source/v2/Meadow.CLI.Core/PackageManager/PackageManager.AssemblyManager.cs index 2d0c4610..2df69f04 100644 --- a/Source/v2/Meadow.CLI.Core/PackageManager/PackageManager.AssemblyManager.cs +++ b/Source/v2/Meadow.CLI.Core/PackageManager/PackageManager.AssemblyManager.cs @@ -6,13 +6,13 @@ public partial class PackageManager public const string PostLinkDirectoryName = "postlink_bin"; public const string PackageOutputDirectoryName = "mpak"; - private string? _meadowAssembliesPath; + private string _meadowAssembliesPath = string.Empty; - private string? MeadowAssembliesPath + private string MeadowAssembliesPath { get { - if (_meadowAssembliesPath == null) + if (string.IsNullOrWhiteSpace(_meadowAssembliesPath)) { // for now we only support F7 // TODO: add switch and support for other platforms var store = _fileManager.Firmware["Meadow F7"]; diff --git a/Source/v2/Meadow.CLI.Core/PackageManager/PackageManager.BuildOptions.cs b/Source/v2/Meadow.CLI.Core/PackageManager/PackageManager.BuildOptions.cs index 190f44c8..412c1c5a 100644 --- a/Source/v2/Meadow.CLI.Core/PackageManager/PackageManager.BuildOptions.cs +++ b/Source/v2/Meadow.CLI.Core/PackageManager/PackageManager.BuildOptions.cs @@ -12,4 +12,4 @@ public record DeployOptions public bool? IncludePDBs { get; set; } } } -} +} \ No newline at end of file diff --git a/Source/v2/Meadow.Cli/AppManager.cs b/Source/v2/Meadow.Cli/AppManager.cs index c52d63d1..edebb7d5 100644 --- a/Source/v2/Meadow.Cli/AppManager.cs +++ b/Source/v2/Meadow.Cli/AppManager.cs @@ -32,7 +32,7 @@ public static async Task DeployApplication( string localBinaryDirectory, bool includePdbs, bool includeXmlDocs, - ILogger logger, + ILogger? logger, CancellationToken cancellationToken) { // TODO: add sub-folder support when HCOM supports it @@ -43,7 +43,7 @@ public static async Task DeployApplication( var dependencies = packageManager.GetDependencies(new FileInfo(Path.Combine(localBinaryDirectory, "App.dll"))); dependencies.Add(Path.Combine(localBinaryDirectory, "App.dll")); - logger.LogInformation("Generating the list of files to deploy..."); + logger?.LogInformation("Generating the list of files to deploy..."); foreach (var file in dependencies) { // TODO: add any other filtering capability here @@ -65,7 +65,7 @@ public static async Task DeployApplication( if (localFiles.Count() == 0) { - logger.LogInformation($"No new files to deploy"); + logger?.LogInformation($"No new files to deploy"); } // get a list of files on-device, with CRCs @@ -85,7 +85,7 @@ public static async Task DeployApplication( // delete those files foreach (var file in removeFiles) { - logger.LogInformation($"Deleting file '{file}'..."); + logger?.LogInformation($"Deleting file '{file}'..."); await connection.DeleteFile(file, cancellationToken); } @@ -94,7 +94,7 @@ public static async Task DeployApplication( { var existing = deviceFiles.FirstOrDefault(f => Path.GetFileName(f.Name) == Path.GetFileName(localFile.Key)); - if (existing != null) + if (existing != null && existing.Crc != null) { if (uint.Parse(existing.Crc.Substring(2), System.Globalization.NumberStyles.HexNumber) == localFile.Value) { @@ -107,7 +107,7 @@ public static async Task DeployApplication( if (!await connection?.WriteFile(localFile.Key, null, cancellationToken)) { - logger.LogWarning($"Error sending'{Path.GetFileName(localFile.Key)}'. Retrying."); + logger?.LogWarning($"Error sending'{Path.GetFileName(localFile.Key)}'. Retrying."); await Task.Delay(100); goto send_file; } diff --git a/Source/v2/Meadow.Cli/Commands/Current/App/AppDeployCommand.cs b/Source/v2/Meadow.Cli/Commands/Current/App/AppDeployCommand.cs index 8ff2b81a..7eee58ae 100644 --- a/Source/v2/Meadow.Cli/Commands/Current/App/AppDeployCommand.cs +++ b/Source/v2/Meadow.Cli/Commands/Current/App/AppDeployCommand.cs @@ -92,7 +92,7 @@ protected override async ValueTask ExecuteCommand() file = new FileInfo(path); } - var targetDirectory = file.DirectoryName; + var targetDirectory = file.DirectoryName!; await AppManager.DeployApplication(_packageManager, connection, targetDirectory, true, false, Logger, CancellationToken); diff --git a/Source/v2/Meadow.Cli/Commands/Current/App/AppRunCommand.cs b/Source/v2/Meadow.Cli/Commands/Current/App/AppRunCommand.cs index 04f997ca..a5542d8d 100644 --- a/Source/v2/Meadow.Cli/Commands/Current/App/AppRunCommand.cs +++ b/Source/v2/Meadow.Cli/Commands/Current/App/AppRunCommand.cs @@ -8,7 +8,7 @@ namespace Meadow.CLI.Commands.DeviceManagement; public class AppRunCommand : BaseDeviceCommand { private readonly IPackageManager _packageManager; - private string _lastFile; + private string? _lastFile; [CommandOption("no-prefix", 'n', IsRequired = false, Description = "When set, the message source prefix (e.g. 'stdout>') is suppressed during 'listen'")] public bool NoPrefix { get; set; } diff --git a/Source/v2/Meadow.Cli/Commands/Current/Cloud/Command/CloudCommandPublishCommand.cs b/Source/v2/Meadow.Cli/Commands/Current/Cloud/Command/CloudCommandPublishCommand.cs index 9ca34e17..54a6e394 100644 --- a/Source/v2/Meadow.Cli/Commands/Current/Cloud/Command/CloudCommandPublishCommand.cs +++ b/Source/v2/Meadow.Cli/Commands/Current/Cloud/Command/CloudCommandPublishCommand.cs @@ -69,7 +69,7 @@ protected override async ValueTask ExecuteCommand() { await CommandService.PublishCommandForCollection(CollectionId, CommandName, Arguments, (int)QualityOfService, Host, CancellationToken); } - else if (DeviceIds.Any()) + else if (DeviceIds != null && DeviceIds.Any()) { await CommandService.PublishCommandForDevices(DeviceIds, CommandName, Arguments, (int)QualityOfService, Host, CancellationToken); } diff --git a/Source/v2/Meadow.Cli/Commands/Current/Cloud/Package/CloudPackageListCommand.cs b/Source/v2/Meadow.Cli/Commands/Current/Cloud/Package/CloudPackageListCommand.cs index 29a0584b..9794c1e9 100644 --- a/Source/v2/Meadow.Cli/Commands/Current/Cloud/Package/CloudPackageListCommand.cs +++ b/Source/v2/Meadow.Cli/Commands/Current/Cloud/Package/CloudPackageListCommand.cs @@ -14,7 +14,7 @@ public class CloudPackageListCommand : BaseCloudCommand public string? OrgId { get; set; } [CommandOption("host", Description = "Optionally set a host (default is https://www.meadowcloud.co)", IsRequired = false)] - public string Host { get; set; } + public string? Host { get; set; } public CloudPackageListCommand( IdentityManager identityManager, diff --git a/Source/v2/Meadow.Cli/Commands/Current/Device/DeviceProvisionCommand.cs b/Source/v2/Meadow.Cli/Commands/Current/Device/DeviceProvisionCommand.cs index 9f155d95..792d9284 100644 --- a/Source/v2/Meadow.Cli/Commands/Current/Device/DeviceProvisionCommand.cs +++ b/Source/v2/Meadow.Cli/Commands/Current/Device/DeviceProvisionCommand.cs @@ -44,7 +44,7 @@ protected override async ValueTask ExecuteCommand() Logger?.LogInformation("Retrieving your user and organization information..."); var userOrgs = await _userService.GetUserOrgs(Host, CancellationToken).ConfigureAwait(false); - if (!userOrgs.Any()) + if (userOrgs == null || !userOrgs.Any()) { Logger?.LogInformation($"Please visit {Host} to register your account."); return; diff --git a/Source/v2/Meadow.Cli/Commands/Current/Firmware/FirmwareDefaultCommand.cs b/Source/v2/Meadow.Cli/Commands/Current/Firmware/FirmwareDefaultCommand.cs index 978ff994..a0b33ef2 100644 --- a/Source/v2/Meadow.Cli/Commands/Current/Firmware/FirmwareDefaultCommand.cs +++ b/Source/v2/Meadow.Cli/Commands/Current/Firmware/FirmwareDefaultCommand.cs @@ -24,7 +24,7 @@ protected override async ValueTask ExecuteCommand() if (Version == null) { - Logger?.LogInformation($"Default firmware is '{collection.DefaultPackage.Version}'."); + Logger?.LogInformation($"Default firmware is '{collection?.DefaultPackage?.Version}'."); } else { diff --git a/Source/v2/Meadow.Cli/Commands/Current/Firmware/FirmwareWriteCommand.cs b/Source/v2/Meadow.Cli/Commands/Current/Firmware/FirmwareWriteCommand.cs index fc220b81..c5b54558 100644 --- a/Source/v2/Meadow.Cli/Commands/Current/Firmware/FirmwareWriteCommand.cs +++ b/Source/v2/Meadow.Cli/Commands/Current/Firmware/FirmwareWriteCommand.cs @@ -30,7 +30,6 @@ public class FirmwareWriteCommand : BaseDeviceCommand private ISettingsManager Settings { get; } private ILibUsbDevice? _libUsbDevice; - private bool _fileWriteError = false; public FirmwareWriteCommand(ISettingsManager settingsManager, FileManager fileManager, MeadowConnectionManager connectionManager, ILoggerFactory loggerFactory) : base(connectionManager, loggerFactory) @@ -201,11 +200,17 @@ private async ValueTask WriteFiles(IMeadowConnection connection, FirmwareType[] }; connection.FileWriteFailed += (s, e) => { - _fileWriteError = true; + Logger?.LogError("Error writing file"); }; var package = await GetSelectedPackage(); + if (package == null) + { + Logger?.LogError($"Firware write failed - unable to find selected package"); + return; + } + var wasRuntimeEnabled = await connection!.Device!.IsRuntimeEnabled(CancellationToken); if (wasRuntimeEnabled) @@ -246,7 +251,7 @@ private async ValueTask WriteFiles(IMeadowConnection connection, FirmwareType[] return; } - if (FirmwareFileTypes.Contains(FirmwareType.ESP)) + if (FirmwareFileTypes != null && FirmwareFileTypes.Contains(FirmwareType.ESP)) { Logger?.LogInformation($"{Environment.NewLine}Writing Coprocessor files..."); diff --git a/Source/v2/Meadow.Cli/Commands/Current/Port/PortSelectCommand.cs b/Source/v2/Meadow.Cli/Commands/Current/Port/PortSelectCommand.cs index 6e2e26af..bf740db6 100644 --- a/Source/v2/Meadow.Cli/Commands/Current/Port/PortSelectCommand.cs +++ b/Source/v2/Meadow.Cli/Commands/Current/Port/PortSelectCommand.cs @@ -50,6 +50,9 @@ private async Task CallConfigCommand(string selectedPort) Settings = new string[] { "route", selectedPort } }; - await setCommand.ExecuteAsync(Console); + if (Console != null) + { + await setCommand.ExecuteAsync(Console); + } } } \ No newline at end of file diff --git a/Source/v2/Meadow.Cli/Commands/Legacy/FlashOsCommand.cs b/Source/v2/Meadow.Cli/Commands/Legacy/FlashOsCommand.cs index 7acc1849..57a9578c 100644 --- a/Source/v2/Meadow.Cli/Commands/Legacy/FlashOsCommand.cs +++ b/Source/v2/Meadow.Cli/Commands/Legacy/FlashOsCommand.cs @@ -54,6 +54,12 @@ protected override async ValueTask ExecuteCommand() { var package = await GetSelectedPackage(); + if (package == null) + { + Logger?.LogError($"Unable to get selected OS package"); + return; + } + var files = new List(); if (!SkipOS) files.Add(FirmwareType.OS); if (!SkipEsp) files.Add(FirmwareType.ESP); @@ -62,7 +68,7 @@ protected override async ValueTask ExecuteCommand() if (Files == null) { - Logger.LogInformation($"Writing all firmware for version '{package.Version}'..."); + Logger?.LogInformation($"Writing all firmware for version '{package.Version}'..."); Files = new FirmwareType[] { @@ -80,7 +86,7 @@ protected override async ValueTask ExecuteCommand() bool deviceSupportsOta = false; // TODO: get this based on device OS version - if (package?.OsWithoutBootloader == null + if (package.OsWithoutBootloader == null || !deviceSupportsOta || UseDfu) { @@ -259,7 +265,14 @@ private async ValueTask WriteFiles() Logger?.LogInformation(message); }; - var package = await GetSelectedPackage(); + + var pack = await GetSelectedPackage(); + + if (pack == null) + { + Logger?.LogError($"Unable to get selected OS package"); + } + FirmwarePackage package = pack!; var wasRuntimeEnabled = await connection.Device.IsRuntimeEnabled(CancellationToken); @@ -275,7 +288,7 @@ private async ValueTask WriteFiles() Console?.Output.Write($"Writing {e.fileName}: {p:0}% \r"); }; - if (Files.Contains(FirmwareType.OS)) + if (Files!.Contains(FirmwareType.OS)) { if (UseDfu) { @@ -288,7 +301,7 @@ private async ValueTask WriteFiles() throw new NotSupportedException("OtA writes for the OS are not yet supported"); } } - if (Files.Contains(FirmwareType.Runtime)) + if (Files!.Contains(FirmwareType.Runtime)) { Logger?.LogInformation($"{Environment.NewLine}Writing Runtime {package.Version}..."); @@ -299,7 +312,7 @@ private async ValueTask WriteFiles() await connection.Device.WriteRuntime(rtpath, CancellationToken); } - if (Files.Contains(FirmwareType.ESP)) + if (Files!.Contains(FirmwareType.ESP)) { Logger?.LogInformation($"{Environment.NewLine}Writing Coprocessor files..."); @@ -313,8 +326,6 @@ private async ValueTask WriteFiles() await connection.Device.WriteCoprocessorFiles(fileList, CancellationToken); } - Logger?.LogInformation($"{Environment.NewLine}"); - if (wasRuntimeEnabled) { await connection.Device.RuntimeEnable(); diff --git a/Source/v2/Meadow.Cloud.Client/Messages/User.cs b/Source/v2/Meadow.Cloud.Client/Messages/User.cs index 1951d6e6..4c6ac125 100644 --- a/Source/v2/Meadow.Cloud.Client/Messages/User.cs +++ b/Source/v2/Meadow.Cloud.Client/Messages/User.cs @@ -7,4 +7,4 @@ public record User public string FirstName { get; set; } public string LastName { get; set; } public string FullName { get; set; } -} +} \ No newline at end of file diff --git a/Source/v2/Meadow.HCom.Integration.Tests/SerialConnectionTests.cs b/Source/v2/Meadow.HCom.Integration.Tests/SerialConnectionTests.cs index 92467a24..707a94d0 100644 --- a/Source/v2/Meadow.HCom.Integration.Tests/SerialConnectionTests.cs +++ b/Source/v2/Meadow.HCom.Integration.Tests/SerialConnectionTests.cs @@ -16,7 +16,7 @@ public void TestInvalidPortName() } [Fact] - public async void TestListen() + public void TestListen() { using (var connection = new SerialConnection(ValidPortName)) { diff --git a/Source/v2/Meadow.Hcom/Connections/SerialConnection.cs b/Source/v2/Meadow.Hcom/Connections/SerialConnection.cs index 698af035..2ca4f621 100644 --- a/Source/v2/Meadow.Hcom/Connections/SerialConnection.cs +++ b/Source/v2/Meadow.Hcom/Connections/SerialConnection.cs @@ -16,10 +16,10 @@ public partial class SerialConnection : ConnectionBase, IDisposable public const int ReadBufferSizeBytes = 0x2000; private const int DefaultTimeout = 5000; - private event EventHandler FileReadCompleted = delegate { }; - private event EventHandler FileWriteAccepted; - private event EventHandler FileDataReceived; - public event ConnectionStateChangedHandler ConnectionStateChanged = delegate { }; + private event EventHandler FileReadCompleted = default!; + private event EventHandler FileWriteAccepted = default!; + private event EventHandler FileDataReceived = default!; + public event ConnectionStateChangedHandler ConnectionStateChanged = default!; private readonly SerialPort _port; private readonly ILogger? _logger; @@ -1178,7 +1178,7 @@ public override async Task GetPublicKey(CancellationToken? cancellationT { var command = RequestBuilder.Build(); - string? contents = null; + string contents = string.Empty; void OnFileDataReceived(object? sender, string data) { diff --git a/Source/v2/Meadow.Hcom/Connections/TcpConnection.cs b/Source/v2/Meadow.Hcom/Connections/TcpConnection.cs index a7b3157f..207d6a4f 100644 --- a/Source/v2/Meadow.Hcom/Connections/TcpConnection.cs +++ b/Source/v2/Meadow.Hcom/Connections/TcpConnection.cs @@ -5,8 +5,8 @@ namespace Meadow.Hcom; public class TcpConnection : ConnectionBase { - private HttpClient _client; - private string _baseUri; + private readonly HttpClient _client; + private readonly string _baseUri; public override string Name => _baseUri; diff --git a/Source/v2/Meadow.Hcom/SerialRequests/RequestBuilder.cs b/Source/v2/Meadow.Hcom/SerialRequests/RequestBuilder.cs index 0fac22e8..c20117a2 100644 --- a/Source/v2/Meadow.Hcom/SerialRequests/RequestBuilder.cs +++ b/Source/v2/Meadow.Hcom/SerialRequests/RequestBuilder.cs @@ -2,7 +2,7 @@ { public static class RequestBuilder { - private static uint _sequenceNumber; + //private static uint _sequenceNumber; public static T Build(uint userData = 0, ushort extraData = 0, ushort protocol = Protocol.HCOM_PROTOCOL_HCOM_VERSION_NUMBER) where T : Request, new() diff --git a/Source/v2/Meadow.SoftwareManager/DownloadFileStream.cs b/Source/v2/Meadow.SoftwareManager/DownloadFileStream.cs index da0f1e66..adad19f6 100644 --- a/Source/v2/Meadow.SoftwareManager/DownloadFileStream.cs +++ b/Source/v2/Meadow.SoftwareManager/DownloadFileStream.cs @@ -6,7 +6,7 @@ namespace Meadow.Software; internal class DownloadFileStream : Stream, IDisposable { - public event EventHandler DownloadProgress; + public event EventHandler DownloadProgress = default!; private readonly Stream _stream; diff --git a/Source/v2/Meadow.SoftwareManager/F7FirmwarePackageCollection.cs b/Source/v2/Meadow.SoftwareManager/F7FirmwarePackageCollection.cs index 9d13bed8..ffac2127 100644 --- a/Source/v2/Meadow.SoftwareManager/F7FirmwarePackageCollection.cs +++ b/Source/v2/Meadow.SoftwareManager/F7FirmwarePackageCollection.cs @@ -11,13 +11,13 @@ namespace Meadow.Software; public class F7FirmwarePackageCollection : IFirmwarePackageCollection { /// - public event EventHandler DownloadProgress; + public event EventHandler DownloadProgress = default!; public event EventHandler DefaultVersionChanged; public string PackageFileRoot { get; } - private List _f7Packages = new(); + private readonly List _f7Packages = new(); public static string DefaultF7FirmwareStoreRoot = Path.Combine( Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), @@ -85,7 +85,7 @@ public Task DeletePackage(string version) // if we're deleting the default, we need to det another default var i = _f7Packages.Count - 1; - while (DefaultPackage.Version == _f7Packages[i].Version) + while (DefaultPackage?.Version == _f7Packages[i].Version) { i--; } diff --git a/Source/v2/Meadow.SoftwareManager/FirmwarePackage.cs b/Source/v2/Meadow.SoftwareManager/FirmwarePackage.cs index ac963c90..339953f9 100644 --- a/Source/v2/Meadow.SoftwareManager/FirmwarePackage.cs +++ b/Source/v2/Meadow.SoftwareManager/FirmwarePackage.cs @@ -11,7 +11,7 @@ internal FirmwarePackage(IFirmwarePackageCollection collection) _collection = collection; } - public string GetFullyQualifiedPath(string file) + public string GetFullyQualifiedPath(string? file) { return Path.Combine(_collection.PackageFileRoot, Version, file); }