Integrate common ISensor interface (net10.0) - #30135
Conversation
|
Hey there @@framinosona! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
jfversluis
left a comment
There was a problem hiding this comment.
84 errors that look similar to this
C:\a_work\1\s\src\Essentials\src\PublicAPI\net\PublicAPI.Shipped.txt(673,1): error RS0017: Symbol 'Microsoft.Maui.Devices.Sensors.IMagnetometer.Stop() -> void' is part of the declared API, but is either not public or could not be found (https://github.com/dotnet/roslyn-analyzers/blob/main/src/PublicApiAnalyzers/PublicApiAnalyzers.Help.md) [C:\a_work\1\s\src\Essentials\src\Essentials.csproj::TargetFramework=net10.0]
C:\a_work\1\s\src\Essentials\src\PublicAPI\net\PublicAPI.Shipped.txt(675,1): error RS0017: Symbol 'Microsoft.Maui.Devices.Sensors.IOrientationSensor.IsMonitoring.get -> bool' is part of the declared API, but is either not public or could not be found (https://github.com/dotnet/roslyn-analyzers/blob/main/src/PublicApiAnalyzers/PublicApiAnalyzers.Help.md) [C:\a_work\1\s\src\Essentials\src\Essentials.csproj::TargetFramework=net10.0]
C:\a_work\1\s\src\Essentials\src\PublicAPI\net\PublicAPI.Shipped.txt(676,1): error RS0017: Symbol 'Microsoft.Maui.Devices.Sensors.IOrientationSensor.IsSupported.get -> bool' is part of the declared API, but is either not public or could not be found (https://github.com/dotnet/roslyn-analyzers/blob/main/src/PublicApiAnalyzers/PublicApiAnalyzers.Help.md) [C:\a_work\1\s\src\Essentials\src\Essentials.csproj::TargetFramework=net10.0]
C:\a_work\1\s\src\Essentials\src\PublicAPI\net\PublicAPI.Shipped.txt(678,1): error RS0017: Symbol 'Microsoft.Maui.Devices.Sensors.IOrientationSensor.Start(Microsoft.Maui.Devices.Sensors.SensorSpeed sensorSpeed) -> void' is part of the declared API, but is either not public or could not be found (https://github.com/dotnet/roslyn-analyzers/blob/main/src/PublicApiAnalyzers/PublicApiAnalyzers.Help.md) [C:\a_work\1\s\src\Essentials\src\Essentials.csproj::TargetFramework=net10.0]
C:\a_work\1\s\src\Essentials\src\PublicAPI\net\PublicAPI.Shipped.txt(679,1): error RS0017: Symbol 'Microsoft.Maui.Devices.Sensors.IOrientationSensor.Stop() -> void' is part of the declared API, but is either not public or could not be found (https://github.com/dotnet/roslyn-analyzers/blob/main/src/PublicApiAnalyzers/PublicApiAnalyzers.Help.md) [C:\a_work\1\s\src\Essentials\src\Essentials.csproj::TargetFramework=net10.0]
|
@jfversluis Of course ... When I add them to Unshipped, I need to remove them in the Shipped ... I just pushed. It should fix it :) |
|
/azp run MAUI-public |
|
Commenter does not have sufficient privileges for PR 30135 in repo dotnet/maui |
|
Ohhh hold on, just discovered there is REMOVED tag ! Will use that instead |
|
@framinosona needs rebase please. Sorry. |
|
@rmarinho Done, should be ready :) |
|
Sorry needs again rebase :( |
Updates sensor and capability interfaces to extend common base interfaces (ISensor and IDeviceCapabilities), promoting reusable design. Removes redundant property and method declarations in specific sensor interfaces by leveraging the base interfaces. Consolidates support detection and monitoring state methods. Improves code maintainability and enforces consistency across device capability-related interfaces.
|
@rmarinho Done :) |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
rmarinho
left a comment
There was a problem hiding this comment.
Seems some api changes twice
/Users/builder/azdo/_work/1/s/src/Essentials/src/PublicAPI/netstandard/PublicAPI.Unshipped.txt(26,1): error RS0025: The symbol 'static Microsoft.Maui.Devices.HapticFeedback.IsSupported.get -> bool' appears more than once in the public API files (https://github.com/dotnet/roslyn/blob/main/src/RoslynAnalyzers/PublicApiAnalyzers/PublicApiAnalyzers.Help.md) [/Users/builder/azdo/_work/1/s/src/Essentials/src/Essentials.csproj::TargetFramework=netstandard2.0]
/Users/builder/azdo/_work/1/s/src/Essentials/src/PublicAPI/netstandard/PublicAPI.Unshipped.txt(27,1): error RS0025: The symbol 'static Microsoft.Maui.Devices.Vibration.IsSupported.get -> bool' appears more than once in the public API files (https://github.com/dotnet/roslyn/blob/main/src/RoslynAnalyzers/PublicApiAnalyzers/PublicApiAnalyzers.Help.md) [/Users/builder/azdo/_work/1/s/src/Essentials/src/Essentials.csproj::TargetFramework=netstandard2.0]
/Users/builder/azdo/_work/1/s/src/Essentials/src/PublicAPI/net/PublicAPI.Unshipped.txt(28,1): error RS0025: The symbol 'static Microsoft.Maui.Devices.HapticFeedback.IsSupported.get -> bool' appears more than once in the public API files (https://github.com/dotnet/roslyn/blob/main/src/RoslynAnalyzers/PublicApiAnalyzers/PublicApiAnalyzers.Help.md) [/Users/builder/azdo/_work/1/s/src/Essentials/src/Essentials.csproj::TargetFramework=net10.0]
/Users/builder/azdo/_work/1/s/src/Essentials/src/PublicAPI/net/PublicAPI.Unshipped.txt(29,1): error RS0025: The symbol 'static Microsoft.Maui.Devices.Vibration.IsSupported.get -> bool' appears more than once in the public API files (https://github.com/dotnet/roslyn/blob/main/src/RoslynAnalyzers/PublicApiAnalyzers/PublicApiAnalyzers.Help.md) [/Users/builder/azdo/_work/1/s/src/Essentials/src/Essentials.csproj::TargetFramework=net10.0]
/Users/builder/azdo/_work/1/s/src/Essentials/src/PublicAPI/netstandard/PublicAPI.Unshipped.txt(26,1): error RS0025: The symbol 'static Microsoft.Maui.Devices.HapticFeedback.IsSupported.get -> bool' appears more than once in the public API files (https://github.com/dotnet/roslyn/blob/main/src/RoslynAnalyzers/PublicApiAnalyzers/PublicApiAnalyzers.Help.md) [/Users/builder/azdo/_work/1/s/src/Essentials/src/Essentials.csproj::TargetFramework=netstandard2.1]
/Users/builder/azdo/_work/1/s/src/Essentials/src/PublicAPI/netstandard/PublicAPI.Unshipped.txt(27,1): error RS0025: The symbol 'static Microsoft.Maui.Devices.Vibration.IsSupported.get -> bool' appears more than once in the public API files (https://github.com/dotnet/roslyn/blob/main/src/RoslynAnalyzers/PublicApiAnalyzers/PublicApiAnalyzers.Help.md) [/Users/builder/azdo/_work/1/s/src/Essentials/src/Essentials.csproj::TargetFramework=netstandard2.1]
0 Warning(s)
6 Error(s)
|
@rmarinho Oops indeed, corrected |
mattleibow
left a comment
There was a problem hiding this comment.
This is overall a good idea, however, I need to check with the language gurus if this is too much of a breaking change. For example, existing libraries using these interfaces need to still work.
Can you confirm that building a library using net8/9 ICompass will work without a recompile?
If not, maybe we can still have these interfaces, just with duplicate methods. Then the ICompass can implement the base interfaces explicitly.
For example:
using System;
public static class C {
public static void Main() {
var c = new Compass();
IDeviceCap cap = c;
ISensor sensor = c;
Console.WriteLine("IsSupported: " + cap.IsSupported);
Console.WriteLine("IsMonitoring: " + sensor.IsMonitoring);
c.Start();
Console.WriteLine("IsMonitoring: " + c.IsMonitoring);
sensor.Stop();
Console.WriteLine("IsMonitoring: " + c.IsMonitoring);
}
}
class Compass : ICompass
{
public bool IsSupported => true;
public bool IsMonitoring { get;private set; }
public void Start()
{
IsMonitoring = true;
}
public void Stop()
{
IsMonitoring = false;
}
}
interface ICompass : ISensor
{
new bool IsMonitoring { get; }
new void Start();
new void Stop();
new bool IsSupported { get; }
bool IDeviceCap.IsSupported => IsSupported;
bool ISensor.IsMonitoring => IsMonitoring;
void ISensor.Start() => Start();
void ISensor.Stop() => Stop();
}
interface ISensor : IDeviceCap
{
bool IsMonitoring { get; }
void Start();
void Stop();
}
interface IDeviceCap
{
bool IsSupported { get; }
}There was a problem hiding this comment.
Pull Request Overview
This PR refactors device capability and sensor interfaces to introduce a unified inheritance hierarchy. It creates common base interfaces (IDeviceCapabilities and ISensor) that eliminate duplicate properties and methods across individual sensor and device capability interfaces, improving code maintainability and consistency.
Key Changes:
- Introduces
IDeviceCapabilitiesbase interface with commonIsSupportedproperty - Introduces
ISensorbase interface extendingIDeviceCapabilitieswith common sensor functionality - Updates all sensor interfaces to inherit from
ISensorinstead of defining duplicate members - Updates device capability interfaces (
IVibration,IHapticFeedback,IPhoneDialer,IAppActions) to inherit fromIDeviceCapabilities
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/Essentials/src/Sensors/IDeviceCapabilities.shared.cs |
New base interface defining common IsSupported property for all device capabilities |
src/Essentials/src/Sensors/ISensors.shared.cs |
New base interface for sensors extending IDeviceCapabilities with monitoring and control methods |
src/Essentials/src/Accelerometer/Accelerometer.shared.cs |
Refactored to inherit from ISensor, removing duplicate properties and methods |
src/Essentials/src/Barometer/Barometer.shared.cs |
Refactored to inherit from ISensor, removing duplicate properties and methods |
src/Essentials/src/Compass/Compass.shared.cs |
Refactored to inherit from ISensor, removing duplicate properties and methods |
src/Essentials/src/Gyroscope/Gyroscope.shared.cs |
Refactored to inherit from ISensor, removing duplicate properties and methods |
src/Essentials/src/Magnetometer/Magnetometer.shared.cs |
Refactored to inherit from ISensor, removing duplicate properties and methods |
src/Essentials/src/OrientationSensor/OrientationSensor.shared.cs |
Refactored to inherit from ISensor, removing duplicate properties and methods |
src/Essentials/src/Vibration/Vibration.shared.cs |
Refactored to inherit from IDeviceCapabilities, removing duplicate IsSupported property |
src/Essentials/src/HapticFeedback/HapticFeedback.shared.cs |
Refactored to inherit from IDeviceCapabilities, removing duplicate IsSupported property |
src/Essentials/src/PhoneDialer/PhoneDialer.shared.cs |
Refactored to inherit from IDeviceCapabilities, removing duplicate IsSupported property |
src/Essentials/src/AppActions/AppActions.shared.cs |
Refactored to inherit from IDeviceCapabilities, removing duplicate IsSupported property |
| PublicAPI files | Updated to reflect the API changes with removed duplicate members and new base interfaces |
|
I have been working on a PR with some non-breaking things: #31086 However, it is a bit unfortunate that it is a bit hacky. Does this new set of interfaces help anything other that duplicate members on the interfaces? Not sure it is worth the breaks in this PR or the (maybe potential) issues in my PR. For now, I am thinking not, but I am open to other opinions and thoughts. |
|
Going to close this one for now and we can re-open if this is something that we see has a strong case. |
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description of Change
Extends sensor interfaces such as IAccelerometer, IBarometer, ICompass, IGyroscope, IMagnetometer, and IOrientationSensor with a common ISensor interface to unify capabilities. Eliminates redundant IsSupported and IsMonitoring properties and Start/Stop methods from individual sensor interfaces. Updates IAppActions, IHapticFeedback, IPhoneDialer, and IVibration to extend from IDeviceCapabilities for uniformity. Improves code maintainability by reducing duplicate functionality.
Relates to codebase cleanup and standardization.
(net9 PR : #30121)