Skip to content
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

Microsoft.Maui.Devices.Flashlight: Check if supported #13458

Closed
vividos opened this issue Feb 20, 2023 · 9 comments · Fixed by #13703
Closed

Microsoft.Maui.Devices.Flashlight: Check if supported #13458

vividos opened this issue Feb 20, 2023 · 9 comments · Fixed by #13703
Labels
area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info fixed-in-8.0.0-preview.3.8149 Look for this fix in 8.0.0-preview.3.8149! proposal/open

Comments

@vividos
Copy link
Contributor

vividos commented Feb 20, 2023

Description

The Microsoft.Maui.Devices.Flashlight (and IFlashlight) API is useful for turning on/off the flashlight on the device's back. Unfortunately, there's no way to check if a flashlight is available in the device at all. When displaying an UI element like a switch, there's no way to hide the UI element when the hardware doesn't support turning on/off.

Public API Changes

My suggestion: A new property in Flashlight and Flashlight:

public interface IFlashlight
{
    bool IsSupported { get; }
}
public static class Flashlight
{
    public static bool IsSupported { get; }
}

The implementation should be fairly easy, since the platform implementations already check if the flashlight is available, and some even have a private IsSupported property.

Intended Use-Case

The use case would be to check if a flashlight is available before offering the user a way to switch on/off the flashlight.

@jsuarezruiz jsuarezruiz added the area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info label Feb 21, 2023
@Ghostbird
Copy link
Contributor

Ghostbird commented Feb 21, 2023

Excellent idea. I'm currently using this workaround:

  static bool? hasFlashlight = null;
  public static async Task<bool> HasFlashlight()
  {
    if (hasFlashlight is null)
    {
      try
      {
        // Trying to turn off the flashlight wil throw if there is no flashlight. I haven't found a better way yet.
        await Flashlight.TurnOffAsync();
        hasFlashlight = true;
      }
      catch
      {
        hasFlashlight = false;
      }
    }
    return (bool)hasFlashlight;
  }

Of course this has the issue that if someone is using the torch while this code runs, it will be turned off. The chance of that happening in my app is extremely slim.

@jfversluis
Copy link
Member

Interesting that we don't have this yet because we have it for most other things 😄

Does every platform offer this detection?

Are you by any chance offering taking this up yourself @vividos? If you are, feel free to do so, small but valuable addition I would say!

@jfversluis jfversluis added this to the Backlog milestone Feb 22, 2023
@ghost
Copy link

ghost commented Feb 22, 2023

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@vividos
Copy link
Contributor Author

vividos commented Feb 22, 2023

Yes, I'll do a spec ticket if you want and can also provide a PR.

@vividos
Copy link
Contributor Author

vividos commented Feb 22, 2023

I checked the source code and I think it's possible to get an answer of IsSupported for every platform

@vividos
Copy link
Contributor Author

vividos commented Feb 22, 2023

@jfversluis I think I found out why Essentials didn't have an IsSupported property: On Windows you can only find out about Lamp objects using async. I think the API must therefore look like this:

public static async Task<bool> GetIsSupported()

Alternative method names could be GetSupported(), CheckSupported() or IsSupportedAsync(). I didn't find a similar method call in Essentials, so I guess you have to suggest something. Should I write a Spec ticket here?

@jfversluis
Copy link
Member

I would imagine there are other places within Essentials where this was the case. Are there? How did we deal with those?

@vividos
Copy link
Contributor Author

vividos commented Feb 24, 2023

I checked every Essentials class; most have a bool IsSupported or bool IsXxxSupported property, and the nearest I could find was the ILauncher method Task<bool> CanOpenAsync(Uri uri); Another alternative method name I suggest: Task<bool> CheckIsSupported().

@Ghostbird
Copy link
Contributor

Ghostbird commented Feb 25, 2023

I personally use Task<bool> IsSupported() for this for all¹ such features, including ones that could be synchronous, using the interface I've called IMaybeSupported. Because IIsSupported just looks weird. My reasoning is:

  • It's convenient to have a single interface for all such things.
  • Checks for supported functionality are relatively rare. I have not yet encountered any situation that where performance could be improved significantly by optimising these kind of calls.
  • I haven't yet run into a situation where I had to make a calling method async specifically because this interface was asynchronous. In my case these kind of calls always ran in contexts that were asynchronous already.
  1. My HasFlashlight example above proves that this is a lie. It doesn't implement that interface yet, but I'll refactor that soon.

@samhouts samhouts added the fixed-in-8.0.0-preview.3.8149 Look for this fix in 8.0.0-preview.3.8149! label Apr 12, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 12, 2023
@samhouts samhouts modified the milestones: Backlog, .NET 8 May 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info fixed-in-8.0.0-preview.3.8149 Look for this fix in 8.0.0-preview.3.8149! proposal/open
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants