Last Comment Bug 754199 - Support Ambient light sensor for Windows 7
: Support Ambient light sensor for Windows 7
Status: NEW
: dev-doc-needed
Product: Core
Classification: Components
Component: Hardware Abstraction Layer (HAL) (show other bugs)
: Trunk
: All Windows 7
: -- normal with 2 votes (vote)
: ---
Assigned To: Makoto Kato [:m_kato]
:
Mentors:
Depends on:
Blocks: 738465
  Show dependency treegraph
 
Reported: 2012-05-11 01:45 PDT by Makoto Kato [:m_kato]
Modified: 2014-08-15 01:01 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix (6.36 KB, patch)
2012-05-14 00:27 PDT, Makoto Kato [:m_kato]
dougt: review+
Details | Diff | Review

Description Makoto Kato [:m_kato] 2012-05-11 01:45:57 PDT
I have the PC that has ambient light sensor...  We can get it using Sensor API (from Windows 7 or later).
Comment 1 Makoto Kato [:m_kato] 2012-05-14 00:27:43 PDT
Created attachment 623602 [details] [diff] [review]
fix
Comment 2 Doug Turner (:dougt) 2012-05-14 22:56:35 PDT
Comment on attachment 623602 [details] [diff] [review]
fix

Review of attachment 623602 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine.  Let me know about the filter.  Fix the other two nits if you like.

::: hal/windows/WindowsSensor.cpp
@@ +118,5 @@
> +    HRESULT hr;
> +    InfallibleTArray<float> values;
> +
> +    // Illuminance level, in lux
> +    hr = aReport->GetSensorValue(SENSOR_DATA_TYPE_LIGHT_LEVEL_LUX, &v);

do you know why they are running a filter over the sensor data in this example:

  http://msdn.microsoft.com/en-us/library/windows/desktop/dd318933%28v=vs.85%29.aspx

@@ +140,5 @@
>  {
> +  // Currently implementation is acceleration and ambient light only.
> +  if ((aSensor == SENSOR_ACCELERATION && sAccelerometer) ||
> +      (aSensor == SENSOR_LIGHT && sLight) ||
> +      (aSensor != SENSOR_ACCELERATION && aSensor != SENSOR_LIGHT)) {

This if statement is kind of hard to follow.  I would have two statements - one that just tests if we aren't SENSOR_ACCELERATION or SENSOR_LIGHT.  Have that return early.  

if (aSensor != SENSOR_LIGHT || aSensor != SENSOR_ACCELERATION) {
  return;
}

if ((aSensor == SENSOR_ACCELERATION && !sAccelerometer) ||
    (aSensor == SENSOR_LIGHT && !sLight)  {
...


Up to you.

@@ +194,5 @@
> +                                   getter_AddRefs(values)))) {
> +      if (SUCCEEDED(values->SetUnsignedIntegerValue(
> +                      SENSOR_PROPERTY_CURRENT_REPORT_INTERVAL,
> +                      DEFAULT_SENSOR_POLL))) {
> +        nsRefPtr<IPortableDeviceValues> returns;

While we are here, lets rename returns to result.
Comment 3 Reuben Morais [:reuben] 2013-02-14 15:51:32 PST
What happened to this patch? Are you still working on it, Makoto?
Comment 4 Makoto Kato [:m_kato] 2013-02-14 17:14:51 PST
ah, I forget this bug.  I will rebase.
Comment 5 Stefan Weiss [:sir_none] 2014-08-07 08:51:55 PDT
Hi Makoto,
again one year over -> patch will come in next time?
Comment 6 Makoto Kato [:m_kato] 2014-08-15 01:01:54 PDT
(In reply to Stefan Weiss [:sir_none] from comment #5)
> Hi Makoto,
> again one year over -> patch will come in next time?

I will work this next week

Note You need to log in before you can comment on or make changes to this bug.