-
-
Notifications
You must be signed in to change notification settings - Fork 47
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.
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
Enable use of firmware archive #290
Conversation
- Extra command line options added to *nanoFirmwareFlasher.Tool* and the implementation to *nanoFirmwareFlasher.Library* to download cloudsmith packages to a firmware archive, list the content of the archive, and deploy firmware from the archive to a device. - Apart from firmware .zip packages, the .dll for the *WIN_DLL_nanoCLR* target can be downloaded and is placed in the archive in such a way if can be used as runtime for *nanoclr.exe*. Same for *WIN32_nanoCLR*, but that is probably not needed as firmware development is probably done in a fork of the nf-interpreter repository and based on the correct version of the CLR. - Added a description of the new command line options to the README.md file - Made some changes to *nanoFirmwareFlasher.Tool* and *nanoFirmwareFlasher.Library* so that the code can be tested: - *nanoFirmwareFlasher.Tests* is a friend of *nanoFirmwareFlasher.Tool* and *nanoFirmwareFlasher.Library*. - Use of new OutputWriter instead of Console. The OutputWriter's output can be collected per unit test even if they are executed in parallel - The location of the firmware archive can be set in unit tests - Added unit tests to *nanoFirmwareFlasher.Tests* to test the affected code. - Use nf-debugger's serial-port protection (GlobalExclusiveDeviceAccess). - Added .editorconfig. This may have resulted in some changes to *nanoFirmwareFlasher.Tool* and *nanoFirmwareFlasher.Library*: update of the file header, use of explicit type instead of `var`.
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (7)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe pull request introduces updates to the Changes
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (49)
.editorconfig
is excluded by none and included by noneREADME.md
is excluded by!**/*.md
and included by nonenanoFirmwareFlasher.Library/CC13x26x2Device.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/CC13x26x2Firmware.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/CC13x26x2Operations.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/Esp32DeviceInfo.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/Esp32Firmware.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/Esp32Operations.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/EspTool.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/ExitCodes.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/FileDeployment/FileDeploymentManager.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/FirmwareArchiveManager.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/FirmwarePackage.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/JLinkCli.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/JLinkDevice.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/JLinkFirmware.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/JLinkOperations.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/NanoDeviceOperations.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/OutputWriter.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/Properties/AssemblyInfo.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/SilinkCli.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/Stm32Firmware.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/Stm32Operations.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/StmDeviceBase.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/StmDfuDevice.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/StmJtagDevice.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/Utilities.cs
is excluded by none and included by nonenanoFirmwareFlasher.Library/nanoFirmwareFlasher.Library.csproj
is excluded by none and included by nonenanoFirmwareFlasher.Tests/CloudsmithApiTests.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tests/FirmwareArchiveManagerTests.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tests/FirmwarePackageTests.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tests/FirmwarePackage_Tests.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tests/Helpers/GetTargetListHelper.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tests/Helpers/OutputWriterHelper.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tests/Helpers/TestDirectoryHelper.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tests/MSTest.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tests/Tool_FirmwareArchiveTests.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tests/nanoFirmwareFlasher.Tests.csproj
is excluded by none and included by nonenanoFirmwareFlasher.Tool/Esp32Manager.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tool/Friends.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tool/NanoDeviceManager.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tool/Options.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tool/Program.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tool/SilabsManager.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tool/Stm32Manager.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tool/TIManager.cs
is excluded by none and included by nonenanoFirmwareFlasher.Tool/nanoFirmwareFlasher.Tool.csproj
is excluded by none and included by nonenanoFirmwareFlasher.sln
is excluded by none and included by nonespelling_exclusion.dic
is excluded by none and included by none
📒 Files selected for processing (3)
- nanoFirmwareFlasher.Library/packages.lock.json (2 hunks)
- nanoFirmwareFlasher.Tool/nanoFirmwareFlasher.Tool.args.json (1 hunks)
- nanoFirmwareFlasher.Tool/packages.lock.json (2 hunks)
🔇 Additional comments (3)
nanoFirmwareFlasher.Tool/nanoFirmwareFlasher.Tool.args.json (1)
1-4
: LGTM: File structure and metadata are well-defined.The JSON structure is correct, and the inclusion of a file version and unique ID is a good practice for configuration management. This will help with versioning and tracking changes to the configuration over time.
nanoFirmwareFlasher.Library/packages.lock.json (1)
39-41
: Approved: nanoFramework.Tools.Debugger.Net package updateThe update of the nanoFramework.Tools.Debugger.Net package from version 2.4.42 to 2.4.45 is consistent with the PR objectives of enhancing the nanoFirmwareFlasher project. This minor version update likely includes backward-compatible new features or bug fixes.
To ensure this update doesn't introduce any unexpected issues, please verify the compatibility of this new version with the rest of the project. You can run the following command to check for any breaking changes or deprecations:
Also applies to: 307-309
✅ Verification successful
Verified: nanoFramework.Tools.Debugger.Net package update
The update of the nanoFramework.Tools.Debugger.Net package to version 2.4.45 has been verified across the project files. All usages are consistent, and no compatibility issues have been detected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any breaking changes or deprecations in the new version # Search for any usage of nanoFramework.Tools.Debugger.Net in the project rg --type csharp "using.*nanoFramework\.Tools\.Debugger" -A 10Length of output: 6577
nanoFirmwareFlasher.Tool/packages.lock.json (1)
19-21
: LGTM! Verify compatibility with the updated debugger version.The update of
nanoFramework.Tools.Debugger.Net
from version 2.4.42 to 2.4.45 looks good. This minor version update should bring improvements or bug fixes to the debugger tool.To ensure compatibility, please run the following script to check for any breaking changes or significant updates in the changelog:
Also applies to: 1099-1099
✅ Verification successful
Verified: No breaking changes detected in nanoFramework.Tools.Debugger.Net v2.4.45.
The update is safe and aligns with project objectives.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for breaking changes or significant updates in nanoFramework.Tools.Debugger.Net changelog # Fetch the changelog or release notes changelog=$(curl -s "https://api.github.com/repos/nanoframework/nf-debugger/releases/tags/v2.4.45") # Check for keywords indicating breaking changes or significant updates if echo "$changelog" | grep -iE "breaking change|major update|significant change"; then echo "Warning: Potential breaking changes or significant updates detected in nanoFramework.Tools.Debugger.Net v2.4.45" echo "Please review the following changelog entries:" echo "$changelog" | grep -iE "breaking change|major update|significant change" -C 2 else echo "No obvious breaking changes or significant updates detected in nanoFramework.Tools.Debugger.Net v2.4.45" fiLength of output: 8124
Some more context about the controlled update strategy and the nanoFramework components involved. This also provides the background for the Discord question about the repository of one of the MSBuild task. It is a bit long, sorry about that, I don't know how to shorten this. Consistency of nanoFramework componentsThere are many components that make up the nanoFramework:
The question which version of the components can be combined (are consistent with each other) is hard to answer. Some components rely on each other, so a new version of one component implies inconsistency with an older version of the other. Some don't, so multiple versions of one component are consistent with multiple versions of another. There is no way to know about that, because there is no documentation on the relations between components, not for humans and certainly not in machine readable formats. Even if there was documentation, there are no tests that verify that consistency. Especially for a "simple" user who relies on the development tools (Visual Studio, NuGet, nanoFramework tools) and has no deep knowledge about the framework, it is impossible to answer the consistency question. The best answer for such a user is: "According to the best knowledge of the nanoFramework contributors, at any given time the components that are current at that moment are consistent with each other." With this in mind, there seem to be only two strategies for a user to work with consistent versions of all components:
The main disadvantage is that the user must always be prepared to do some work: update all components and/or update the user's code if updates come with breaking changes. Someone with a program manager's hat may categorize the nanoFramework as an uncontrollable risk for the success of a project, as it is impossible to schedule/prepare for the work that results from an auto-update. The only other strategy that seems to be possible is an answer to that:
It is not sufficient to use only current components. The NuGet packages that are used must be compatible with the firmware of the device the software is to be deployed to. That is important to keep in mind for both strategies. But as the controlled update strategy is driven by I don't want any surprises, it is important for that strategy that the compatibility of NuGet packages and firmware can be verified as early as possible. Controlled update: requirements and solutions for nanoFrameworkThe controlled update strategy is not yet supported by the nanoFramework, because for a simple user it is hard to freeze and work with a frozen version of the nanoFramework. It may even not be possible at all. What a user should be able to do, per component:
Summary: PRTo get the controlled update strategy working, there are going to be several PRs:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass on review.
Overall, it looks good.
Too name changes and most of them "hide" the real and relevant ones. That's why it's preferreable to have atomic and focused PRs.
@frobijn your description of the controlled update motivation and proposed solutions, despite valid, will get "lost" in this PR... |
Are you saying I should have split this up into multiple PR? E.g,. testability improvement. locking, extra options? That is not always easy. The work on the options came first, quite a while ago, but when I started to add unit tests I stumbles upon the testability issues. It's hard to work on multiple branches at the same time.. |
Please do, I didn't find a suitable place for that. Perhaps also make a document for the test framework v3. |
@frobijn URLs for the RFCs docs emailed to you. |
- Extra command line options added to *nanoFirmwareFlasher.Tool* and the implementation to *nanoFirmwareFlasher.Library* to download cloudsmith packages to a firmware archive, list the content of the archive, and deploy firmware from the archive to a device. - Apart from firmware .zip packages, the .dll for the *WIN_DLL_nanoCLR* target can be downloaded and is placed in the archive in such a way if can be used as runtime for *nanoclr.exe*. Same for *WIN32_nanoCLR*, but that is probably not needed as firmware development is probably done in a fork of the nf-interpreter repository and based on the correct version of the CLR. - Added a description of the new command line options to the README.md file - Made some changes to *nanoFirmwareFlasher.Tool* and *nanoFirmwareFlasher.Library* so that the code can be tested: - *nanoFirmwareFlasher.Tests* is a friend of *nanoFirmwareFlasher.Tool* and *nanoFirmwareFlasher.Library*. - Use of new OutputWriter instead of Console. The OutputWriter's output can be collected per unit test even if they are executed in parallel - The location of the firmware archive can be set in unit tests - Added unit tests to *nanoFirmwareFlasher.Tests* to test the affected code. - Use nf-debugger's serial-port protection (GlobalExclusiveDeviceAccess). - Added .editorconfig. This may have resulted in some changes to *nanoFirmwareFlasher.Tool* and *nanoFirmwareFlasher.Library*: update of the file header, use of explicit type instead of `var`.
@josesimoes all changes from review comments applied, plus exclusive access. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for adding this. And for the rest of the improvements, of course. 👍🏻
Seems that this PR broke the ability to run the tool on Linux platform. Is there anything obvious here? |
Description
The changes in this repository implement the firmware archive feature:
Extra command line options added to nanoFirmwareFlasher.Tool and the implementation to nanoFirmwareFlasher.Library to download cloudsmith packages to a firmware archive, list the content of the archive, and deploy firmware from the archive to a device.
Apart from firmware .zip packages, the .dll for the WIN_DLL_nanoCLR target can be downloaded and is placed in the archive in such a way if can be used as runtime for nanoclr.exe. Same for WIN32_nanoCLR, but that is probably not needed as firmware development is probably done in a fork of the nf-interpreter repository and based on the correct version of the CLR.
Added a description of the new command line options to the README.md file
Made some changes to nanoFirmwareFlasher.Tool and nanoFirmwareFlasher.Library so that the code can be tested:
Added unit tests to nanoFirmwareFlasher.Tests to test the affected code. There are still 4 do-nothing-tests that are left untouched.
Use of nf-debugger's serial-port protection (GlobalExclusiveDeviceAccess).
Added .editorconfig. This may have resulted in some changes to nanoFirmwareFlasher.Tool and nanoFirmwareFlasher.Library: update of the file header, use of explicit type instead of
var
.Motivation and Context
The current version of the .NET nanoFramework is based on "auto-update everything" versioning: make sure that you always use the latest version of NuGet packages, firmware and tools, and you can be sure that the packages and tools are consistent with each other.
This may be fine if there never are breaking changes in the .NET nanoFramework, and if it is always possible to update the firmware on any device. But that is not the case. E.g., recent move to littlefs was a breaking change: it would have broken some low-level libraries of this contributor (File.GetLastWriteTime disappeared). If nanoclr.exe is auto-updated the Virtual nanoDevice is no longer consistent with the libraries. If firmware is installed on a new device, that firmware will also be different.
What is needed is a versioning strategy where the user of the framework (and not the framework's core team) is in charge of updating the framework components used by the project. In this controlled update strategy the user works with a frozen version of the framework and has a guarantee very early in the development process that the final deployment to a device will succeed. The user decides if and when to update to a next version of the framework.
The auto-update and the controlled update strategies are described in a getting started guide (concept). This guide will be made available once the PR of all required components have been accepted. Apart from this PR, there will also be a PR for the MSBuild task/NuGet package. (See the first comment below for a more elaborate description.)
The use of a firmware archive is added to Packaging, versioning and deployment (concept) in the architecture section. A PR for this page will be created after this PR has been accepted.
How Has This Been Tested?
Unit tests have been added for all code that is affected by this change, except for the tasks that require a real hardware device (e.g., GlobalExclusiveDeviceAccess). Those have been tested by running the tool for a connected device.
Types of changes
Checklist:
Summary by CodeRabbit
New Features
nanoFirmwareFlasher
tool, allowing users to specify command-line arguments for various operations.nanoFramework.Tools.Debugger.Net
to ensure compatibility with the latest features.Bug Fixes
nanoFramework.Tools.Debugger.Net
package.