Sensor API has accelerometer supports. We should use this.

sensorapi.lib has CLSID information for Sensor API, so we have to link it. But, since there is no export function, linked binary works on 2000 and XP.

Comment on attachment 519369 [details] [diff] [review] fix v2 Thanks for jumping on this! Could you use CComQIPtr instead of raw pointers. They are similar to our nsCOMPtr's and will save you from worrying about releasing. throw up another patch. i think you are very close.

(In reply to comment #4) > Comment on attachment 519369 [details] [diff] [review] > fix v2 > > Thanks for jumping on this! > > Could you use CComQIPtr instead of raw pointers. They are similar to our > nsCOMPtr's and will save you from worrying about releasing. Doug, could we use ATL? CComQIPtr is ATL's class.

Kinda related - shouldn't nsAccelerometerSystem be moved to dom?

Maybe, but please don't move files before hg is fixed http://mercurial.selenic.com/bts/issue1576

jimm, probably /dom/system. Do you know the answer to Makoto's question? Also, how do I test this? Other than getting-a-windows-laptop...

(In reply to comment #9) > jimm, probably /dom/system. Do you know the answer to Makoto's question? I've never used ATL in mozilla code, I use our own wrappers. (I believe in at least one build configs, the headers aren't available.)

(In reply to comment #9) > > Also, how do I test this? Other than getting-a-windows-laptop... You have to get Laptop that has accelerometer with Sensor API such as VAIO P (http://www.sonystyle.com/webapp/wcs/stores/servlet/ProductDisplay?catalogId=10551&storeId=10151&langId=-1&productId=8198552921666173660). This code is tested by my VAIO P. Doug, Should I change reviewer to anyone? jimm? Also, the latest fix uses nsRefPtr instead of CComQIPtr. Although I don't think nsRefPtr is best for Windows COM, it is used by netwerk/system/win32 for same reason.

Could you perhaps provide tryserver builds so that people could try out the patch? (I don't have dev tools on Windows so can't compile myself. I wonder if my Vaio VPC-SB1A9E has accelerometer.)

Comment on attachment 522335 [details] [diff] [review] fix v3 the patch looks fine, but: 1) I am unsure about the build changes -- ted or some build peer should be consulted. 2) I am not sure nsRefPtr is the best substitue for CComQIPtr, but if it works, great. 3) I am unable to verify that the axis data is correct since I do not have a similar laptop. Please ensure that the demo's behave properly. Lastly, do you have access to a gyroscope on the laptop you have -- is there a Window 7 defined sensor for that?

Humm, due to bug 615597, this fix doesn't work. When I submit a fix for reviewing, it works fine. I think that new DeviceOrientation code may not work on other platforms except to Android.

Why does using a COM API require linking to an import library?

(In reply to comment #15) > Why does using a COM API require linking to an import library? CLSID_SensorManager isn't include in uuid.lib. It is into sensorapi.lib If we define CLSID_SensorManager into our source, we doesn't need import library.

fwiw I'd prefer that we defined the CLSID in our own source rather than needing to do this build hackery to link sensorapi.lib.

(In reply to comment #17) > fwiw I'd prefer that we defined the CLSID in our own source rather than > needing to do this build hackery to link sensorapi.lib. Kyle, how about "#pragma comment(lib, "sensorsapi.lib")" instead of? If not linking sensorspai.lib, I have to add the following. const CLSID CLSID_SensorManager = ...; const IID IID_ISensorManager = ...; REFSENSOR_TYPE_ID SENSOR_TYPE_ACCELEROMETER_3D = ...; REFPROPERTYKEY SENSOR_DATA_TYPE_X_G = ...; REFPROPERTYKEY SENSOR_DATA_TYPE_Y_G = ...; REFPROPERTYKEY SENSOR_DATA_TYPE_Z_G = ...;

Sure, that works too.

(In reply to comment #19) > Sure, that works too. Sensorsapi.dll is only available on Windows 7.

(In reply to comment #20) > (In reply to comment #19) > > Sure, that works too. > > Sensorsapi.dll is only available on Windows 7. See comment #16. This is static library. This DLL is loaded via COM and this #pragma doesn't import any global values and functions. So, the binary that has this fix works on XP/Vista. (Of course XP and Vista don't have Sensor API and DLL).

(In reply to comment #21) > (In reply to comment #20) > > (In reply to comment #19) > > > Sure, that works too. > > > > Sensorsapi.dll is only available on Windows 7. > > See comment #16. This is static library. This DLL is loaded via COM and > this #pragma doesn't import any global values and functions. > > So, the binary that has this fix works on XP/Vista. (Of course XP and Vista > don't have Sensor API and DLL). Ah, sorry, should have read back farther in the history.

Comment on attachment 522335 [details] [diff] [review] fix v3 Review of attachment 522335 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/build/Makefile.in @@ +289,5 @@ > OS_LIBS += -framework OpenGL > endif > > +ifeq ($(OS_ARCH),WINNT) > +ifneq (,$(filter-out 05020000 06000000,$(MOZ_WINSDK_TARGETVER))) This is okay, but we should probably just drop support for anything older than the Windows 7 SDK at this point. People have had plenty of time to transition. (You don't have to do so as part of this patch.)

try server build ftp://ftp.mozilla.org/pub/firefox/try-builds/m_kato@ga2.so-net.ne.jp-e57bf881795b/try-win32/ You may need sensor API driver. I don't know that sony releases it for all regions.

Hi guys, I just want to tell you about this thing I found: http://feishare.com/latest/remotesensors-development-detail It's an app for Android together with server software for Windows. You install the driver for Windows and run the server, then you connect the phone to it with the app over WLAN or Bluetooth. Then you get access to the phone's sensors in Windows. I have tried it, and accelerators work (there's a demo on that site, and it's working). Essentially that could turn our phones into Wiimote-like game controllers for HTML5 games. :)

rewriting for hal. - build env of min requirement is Windows 7 SDK. so I don't need SDK check - adding library is static library that has GUIDs only. (So built binary works on Windows XP, but Windows XP has no sensor API, sensor doesn't work.)

Comment on attachment 612119 [details] [diff] [review] fix v4 Review of attachment 612119 [details] [diff] [review]: ----------------------------------------------------------------- ::: hal/windows/WindowsSensor.cpp @@ +147,5 @@ > + IID_IPortableDeviceValues, > + getter_AddRefs(values)))) { > + if (SUCCEEDED(values->SetUnsignedIntegerValue( > + SENSOR_PROPERTY_CURRENT_REPORT_INTERVAL, > + DEFAULT_SENSOR_POLL))) { Seems like this would generate a lot of event traffic on a device that supports the sensor and the UI is subscribed to these events. Maybe we should only send events if the the value has actually changed? We'll be using some of this code for Win8 metro. For winrt Microsoft had to implement something similar to this - http://msdn.microsoft.com/en-us/library/windows/apps/windows.devices.sensors.accelerometer.aspx

(In reply to Jim Mathies [:jimm] from comment #27) > Comment on attachment 612119 [details] [diff] [review] > fix v4 > > Review of attachment 612119 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: hal/windows/WindowsSensor.cpp > @@ +147,5 @@ > > + IID_IPortableDeviceValues, > > + getter_AddRefs(values)))) { > > + if (SUCCEEDED(values->SetUnsignedIntegerValue( > > + SENSOR_PROPERTY_CURRENT_REPORT_INTERVAL, > > + DEFAULT_SENSOR_POLL))) { > > Seems like this would generate a lot of event traffic on a device that > supports the sensor and the UI is subscribed to these events. Maybe we > should only send events if the the value has actually changed? This setting is for the sensor associated by ISensor. So this code is that event is sent by accelerometer only per setting interval. Also ,some drivers may not be able to set 100ms because min internal of sensor driver is over 100ms. Should we change to nsITimer implementation? > We'll be using some of this code for Win8 metro. For winrt Microsoft had to > implement something similar to this - > > http://msdn.microsoft.com/en-us/library/windows/apps/windows.devices.sensors. > accelerometer.aspx ISensor is on Desktop App only, so we need metro specific implementation for accelerometer that you point.

Comment on attachment 612119 [details] [diff] [review] fix v4 Review of attachment 612119 [details] [diff] [review]: ----------------------------------------------------------------- ::: hal/windows/WindowsSensor.cpp @@ +72,5 @@ > + hr = aReport->GetSensorValue(SENSOR_DATA_TYPE_ACCELERATION_X_G, &v); > + if (FAILED(hr)) { > + return hr; > + } > + values.AppendElement(float(v.dblVal * 10)); Why are you multiplying by 10 here? Your units should be in m/s^2. Orientation looks like http://developer.android.com/images/axis_device.png Same for the other values.

(In reply to Makoto Kato from comment #28) > > This setting is for the sensor associated by ISensor. So this code is that > event is sent by accelerometer only per setting interval. Still seems like you are asking for a constant stream of sensor events, every 100msec, regardless if the value has changed. > > Also ,some drivers may not be able to set 100ms because min internal of > sensor driver is over 100ms. Should we change to nsITimer implementation? > > > > We'll be using some of this code for Win8 metro. For winrt Microsoft had to > > implement something similar to this - > > > > http://msdn.microsoft.com/en-us/library/windows/apps/windows.devices.sensors. > > accelerometer.aspx > > ISensor is on Desktop App only, so we need metro specific implementation for > accelerometer that you point. Are you sure about that? I'd be willing to bet the winrt apis are calling the same com interfaces we're calling here, in which case this interface would work for both. (Technically in metro we are still a desktop app, it's just that we are granted special privileges to access metro.) Once this lands I can test to see.

(In reply to Doug Turner (:dougt) from comment #29) > Why are you multiplying by 10 here? Your units should be in m/s^2. > Orientation looks like http://developer.android.com/images/axis_device.png Ahh, I misunderstand this. It should be 9.8...

(In reply to Jim Mathies [:jimm] from comment #30) > > ISensor is on Desktop App only, so we need metro specific implementation for > > accelerometer that you point. > > Are you sure about that? I'd be willing to bet the winrt apis are calling > the same com interfaces we're calling here, in which case this interface > would work for both. > > (Technically in metro we are still a desktop app, it's just that we are > granted special privileges to access metro.) Really? MSDN says that this is desktop only. I don't test on Metro base code yet. > Once this lands I can test to see. When isn't working, I will write WinRT sytle code after landing.

Also, on Win8/arm target, there is no some libs for sensors. So we need WinRT C++ code for arm target.

Comment on attachment 613220 [details] [diff] [review] v5 Review of attachment 613220 [details] [diff] [review]: ----------------------------------------------------------------- ::: hal/windows/WindowsSensor.cpp @@ +87,5 @@ > + hr = aReport->GetSensorValue(SENSOR_DATA_TYPE_ACCELERATION_Z_G, &v); > + if (FAILED(hr)) { > + return hr; > + } > + values.AppendElement(float(-v.dblVal * MEAN_GRAVITY)); can you confirm that you need to use the -1 here and on Y

(In reply to Doug Turner (:dougt) from comment #35) > can you confirm that you need to use the -1 here and on Y Yes. This value of Windows 7 sensoer api is reversed.