Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
4 open findings
Replace("-BL", "")removes all occurrences of-BL, not just a-BLsuffix. If a board name… · NewBootloaderStrsupport is newly introduced but isn’t covered by the added unit tests. Adding at… · Newnew Version()sorts invalid/unparseable version strings before real versions (since it becomes… · Newnew string[0]allocates each time this path is hit. Since these are empty-array defaults, use… · New
What changed in this PR
Removes Xamarin.Forms dependencies from the Windows build by deleting Xamarin references/projects and replacing the Xamarin-based firmware selection UI with a WinForms implementation.
Changes:
- Removed Xamarin.Forms/Xamarin.Essentials references (projects, packages, and related helper/using code).
- Replaced the Xamarin firmware selection dialog with a new WinForms
FirmwareSelectionForm. - Added a new
FirmwareFilterfaceted-filter utility plus MSTest coverage.
| File | Description |
|---|---|
| Utilities/ExtensionsMP.cs | Removes Xamarin WinForms renderer helper and related usings. |
| Program.cs | Avoids Xamarin.Essentials usage in non-LIB builds; keeps desktop font scaling config. |
| Plugins/example23-switch.cs | Removes Xamarin.Forms platform configuration import. |
| GCSViews/ConfigurationView/ConfigFirmwareManifest.cs | Switches firmware selection UX from Xamarin page to WinForms form. |
| MissionPlanner.csproj | Drops Xamarin.Forms/Xamarin.Essentials package references and WinForms platform project reference. |
| MissionPlanner.sln | Removes Xamarin.Forms.Platform.WinForms project from the solution. |
| ExtLibs/Xamarin/Xamarin/Xamarin.csproj | Removes old linked test XAML file and updates embedded-resource generator mapping. |
| ExtLibs/Strings/Strings.resx | Adds localized strings for the new firmware selection form. |
| ExtLibs/ArduPilot/FirmwareFilter.cs | Adds new faceted filtering implementation for firmware manifest entries. |
| Controls/FirmwareSelectionForm.cs | Adds WinForms UI for selecting firmware via filters and a results grid. |
| MissionPlannerTests/ArduPilot/FirmwareFilterTests.cs | Adds unit tests for firmware filtering/options behavior. |
Files not reviewed (1)
- ExtLibs/Strings/Strings.Designer.cs: Generated file
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| if (knownDevice) | ||
| { | ||
| var platform = device.Value.board.Replace("-BL", ""); |
Comment on lines
+21
to
+23
| BoardId, | ||
| BootloaderStr | ||
| } |
Comment on lines
+82
to
+83
| return values.OrderBy(a => { Version v; return Version.TryParse(a, out v) ? v : new Version(); }) | ||
| .ThenBy(a => a, StringComparer.Ordinal).ToList(); |
Comment on lines
+119
to
+123
| return fw.Usbid ?? new string[0]; | ||
| case Field.BoardId: | ||
| return new[] { fw.BoardId.ToString() }; | ||
| case Field.BootloaderStr: | ||
| return fw.BootloaderStr ?? new string[0]; |
Copilot stopped reviewing on behalf of
meee1 due to an error
October 14, 2026 04:15
Pure filtering logic extracted from the Xamarin FirmwareSelection page so it can be unit tested and reused by a WinForms replacement. Options for each field are computed from the other fields' selections, so a choice can be widened again, and the format filter is now applied. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JCrzU8rJurW3DPH4UAvTWR
FirmwareSelectionForm uses FirmwareFilter to offer faceted dropdowns and a results grid (platform, vehicle, release type, version, format, file) instead of raw URLs. Detected boards keep the existing simplified view: platform is preselected and only platform/release type/version filters are shown, the latter two only when there is a choice. The "all options" path now shows every filter rather than hiding them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JCrzU8rJurW3DPH4UAvTWR
The page is now only used by the Android/Xamarin Firmware screen, which already linked it. Move it beside that screen and drop the WinForms hosting helper ShowXamarinControl, so the shared root sources no longer use Xamarin.Forms. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JCrzU8rJurW3DPH4UAvTWR
- drop the Xamarin.Forms/Xamarin.Essentials packages and the Xamarin.Forms.Platform.WinForms project reference from MissionPlanner.csproj - remove Xamarin.Forms.Platform.WinForms from MissionPlanner.sln; the folder stays, as MissionPlannerLib compiles it for the Android/Mac apps - Program.cs: desktop always resolved to Desktop/Unknown idiom, so disable ZedGraph font scaling directly; keep the Essentials check under LIB - example23-switch: remove an unused Xamarin using that would fail to compile at runtime once the assembly is no longer shipped Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JCrzU8rJurW3DPH4UAvTWR
DataGridView selects its first row when shown, which enabled Upload before the user had chosen. Also let the prompt wrap on narrow windows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JCrzU8rJurW3DPH4UAvTWR
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XoULMVVcPGs6rvDkA6FAyN
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XoULMVVcPGs6rvDkA6FAyN
…here is a choice The "all options" path passes the whole manifest for every detected port, so a known board could list every vehicle type with no way to filter by it. On the normal path the list is already one vehicle type, so nothing changes there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XoULMVVcPGs6rvDkA6FAyN
- FirmwareSelectionForm: a board in its bootloader reports <platform>-BL; strip only that suffix instead of every -BL in the name - FirmwareFilter: use Array.Empty<string>() for missing UsbId/BootloaderStr - FirmwareFilterTests: cover BootloaderStr options and matching, including entries with no or null bootloader strings Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XoULMVVcPGs6rvDkA6FAyN
meee1
force-pushed
the
ccr-fad59d5e-ire1ur
branch
from
October 20, 2026 20:00
2b3f7df to
1df0a88
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.