Conversation
cad6c41 to
596c442
Compare
596c442 to
f585753
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR initializes a new DeviceCore library for cross-platform device functionality on Uno/WinUI applications. The PR adds comprehensive device service abstractions and implementations for Android, iOS, and Windows platforms.
- Establishes foundational library structure with abstractions and platform-specific implementations
- Implements core device services including accelerometer, ambient light, battery information, flashlight, and screen wake lock
- Sets up CI/CD pipeline with Azure DevOps and comprehensive build configuration
Reviewed Changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| DeviceCore.sln | Solution structure defining library projects and build configuration |
| src/lib/DeviceCore.Abstractions/ | Core abstractions and interfaces for device services with fake implementations for testing |
| src/lib/DeviceCore.Uno.WinUI/ | Platform-specific implementations using Uno framework for Android/iOS/Windows |
| build/ | Complete CI/CD pipeline configuration with Azure DevOps, GitVersion, and NuGet publishing |
| README.md | Comprehensive documentation covering setup, usage examples, and platform-specific requirements |
src/lib/DeviceCore.Abstractions/BatteryInformation/PowerSupplyStatus.cs
Outdated
Show resolved
Hide resolved
src/lib/DeviceCore.Uno.WinUI/BatteryInformation/EnergySaverStatus.Extensions.cs
Outdated
Show resolved
Hide resolved
src/lib/DeviceCore.Abstractions/AmbientLight/AmbiantLightReading.cs
Outdated
Show resolved
Hide resolved
src/lib/DeviceCore.Abstractions/AmbientLight/IAmbientLightProvider.cs
Outdated
Show resolved
Hide resolved
src/lib/DeviceCore.Abstractions/AmbientLight/FakeAmbientLightProvider.cs
Outdated
Show resolved
Hide resolved
src/lib/DeviceCore.Abstractions/BatteryInformation/FakeBatteryInformationProvider.cs
Outdated
Show resolved
Hide resolved
src/lib/DeviceCore.Abstractions/BatteryInformation/FakeBatteryInformationProvider.cs
Outdated
Show resolved
Hide resolved
f585753 to
e5d4f3d
Compare
e5d4f3d to
5213d30
Compare
5213d30 to
8ee0f55
Compare
493a4ce to
f105cf8
Compare
bee174c to
94766c9
Compare
94766c9 to
fe0e055
Compare
| /// <summary> | ||
| /// Gets or sets the current report interval for the accelerometer. | ||
| /// </summary> | ||
| uint ReportInterval { get; set; } |
| /// Gets the performance count associated with the reading. | ||
| /// This allows the reading to be synchronized with other devices and processes on the system. | ||
| /// </summary> | ||
| public TimeSpan? PerformanceCount { get; } |
There was a problem hiding this comment.
I don't understand this property. It's named like count (so some kind of integer?) but it's a nullable TimeSpan and it's not clear what the value means.
| /// <summary> | ||
| /// Gets the performance count associated with the reading.This allows the reading to be synchronized with other devices and processes on the system. | ||
| /// </summary> | ||
| public TimeSpan? PerformanceCount { get; } |
There was a problem hiding this comment.
Same here, what does it mean?
| /// <summary> | ||
| /// Gets the total percentage of charge remaining from all batteries connected to the device. | ||
| /// </summary> | ||
| int RemainingChargePercent { get; } |
There was a problem hiding this comment.
I would expect a percentage to be a value between 0.0 and 1.0 but this is an int. Does it mean that the value is between 0 and 100?
| /// Gets or sets the brightness of the flashlight with a value in the [0.0, 1.0] range. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// On Android, flashlight brightness cannot be controlled, hence any non-zero BrightnessLevel results in the full brightness of the flashlight. |
There was a problem hiding this comment.
You mention BrightnessLevel but the property name is Brightness. Are you referring to something else?
| /// <remarks> | ||
| /// You must set the <see cref="Brightness"/> property before or after calling this method to ensure the desired brightness level is applied when the flashlight is turned on. | ||
| /// </remarks> | ||
| void Toggle(); |
There was a problem hiding this comment.
How do we know if we turn it on or off? Why not have a TurnOn(float brightness) and TurnOff methods?
There was a problem hiding this comment.
It's also weird that you have to set the brightness before turning on... It means that if the light is already on, it cannot be adjusted? If that's the case, it might not be worth having a setter on the Brightness property.
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="10.0.1" /> |
There was a problem hiding this comment.
Do you actually need Logging.Abstractions here?
| <Company>nventive</Company> | ||
| <AssemblyName>DeviceCore.Uno.WinUI</AssemblyName> | ||
| <PackageId>DeviceCore.Uno.WinUI</PackageId> | ||
| <Description>Provides cross-platform device APIs for Uno Platform and WinUI, supporting .NET 8, Android, iOS, and Windows targets.</Description> |
There was a problem hiding this comment.
You support .NET 10, not 8.
| @@ -0,0 +1,67 @@ | |||
| | |||
There was a problem hiding this comment.
Now that Visual Studio 2026 is out, you could update this to the new format.
| /// <summary> | ||
| /// The fake implementation of <see cref="IAccelerometerService"/> for testing purposes. | ||
| /// </summary> | ||
| public sealed class FakeAccelerometerService : IAccelerometerService |
There was a problem hiding this comment.
What's the vision/strategy for the fake implementations? Right now it seems like a bare minimum implementation that doesn't mimic the real thing. In this one, the observable sequences complete after 1 value, which is likely not the case with the real implementation. Also the ReportInterval property is not even used.
GitHub Issue: #
Proposed Changes
What is the current behavior?
Empty repository.
What is the new behavior?
Added documentation, CI/CD, and some features.
Checklist
Please check that your PR fulfills the following requirements:
Automated tests for the changes have been added/updated.Other information