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

Sensors API #227

Closed
wants to merge 1 commit into from
Closed

Sensors API #227

wants to merge 1 commit into from

Conversation

ericpearson
Copy link

This implements the sensors on the BB10 device, all currently present sensors are supported.

Issue #301

Docs are here:
blackberry-webworks/WebWorks-API-Docs/pull/218

@nukulb
Copy link

nukulb commented Oct 26, 2012

perhaps we can do some work on the interface

I do know this exists
http://dvcs.w3.org/hg/dap/raw-file/tip/sensor-api/Overview.html#datatypes

@Pagey - can you check if any of this is supported by webkit? my guess is its not.

@@ -11,24 +11,7 @@


<!-- include spec files here... -->
<script type="text/javascript" src="spec/blackberry.app.js"></script>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason to remove all of these

@nukulb
Copy link

nukulb commented Oct 26, 2012

@jeffheifetz @kwallis - need input on this-
Some open questions are -

  1. since there is a spec around Sensors API, should we meeting that spec or designing close to that spec.
    I think we should but the spec seems very limited.
    http://dvcs.w3.org/hg/dap/raw-file/tip/sensor-api/Overview.html
    @jeffheifetz any suggestions on how to retro fit our sensors to be closer to the spec.
  2. Accelerometer and device orientation are already supported by webkit, should we expose these through the interface or not?

@kwallis
Copy link

kwallis commented Oct 26, 2012

  1. We had talked about doing investigation on what already exists out there in specs before we implemented, did that not happen? For instance, looking at the link you sent, it indicated it was put on hold as they pulled specific sensor APIs out into their own specs. Looking at http://www.w3.org/2009/dap/, there are specs for proximity, ambient light, temperature, humidity, and pressure.
  2. If the browser supports the API already, this should be a pure documentation task, and not exposed in a separate WebWorks API

@nukulb
Copy link

nukulb commented Oct 26, 2012

They are really same spec just with different names For eg.
interface DeviceProximityEvent : Event {
readonly attribute double value;
readonly attribute double min;
readonly attribute double max;
};

interface AmbientTemperatureEvent : Event {
readonly attribute double f;
readonly attribute double c;
readonly attribute double k;
};

Some have one value and other have more than 1.

So can we define the interfaces now for each of the sensors, seems like

You can look at the file below to see what all sensors are possible and what they return.
4181e1d#L10R163

Question is whether you want to ployfill like the Notifications APIs or do you want to provide all the APIs under
blackberry.addEventListener.

It really doesn't matter much to me but a decision needs to be made.

I think for the ones that defined we can simply meet the spec definition.

There are APIs where we return more than the spec in that case we can append to it.

@nukulb
Copy link

nukulb commented Oct 26, 2012

updated the above comment to put proper link in there.

@kwallis
Copy link

kwallis commented Oct 26, 2012

I would like to meet the spec definitions if we can, and extend where we have additional features/requirements

@nukulb
Copy link

nukulb commented Oct 26, 2012

temperature, humidity,altimeter and pressure are not supported

4181e1d#L10R234

So the only overlap is proximity, ambient light, but we should have similar interfaces for all the sensors to make sure it doesn't go

@kwallis
Copy link

kwallis commented Oct 26, 2012

We have temperature, humidity, and altimeter listed as available in the original user story, with links to the PPS objects:
blackberry#301
So I am not sure what you mean by they are not supported?

@nukulb
Copy link

nukulb commented Oct 26, 2012

updated

@ericpearson
Copy link
Author

I agree now that looking the w3c spec it should be better to align ourselves with it, when I originally designed this I wasn't aware of any of the events other than the devicemotion and deviceorientation events. I guess for holster we can have a deviceholstered event listener. I still find the w3c very cryptic to understand what exactly is supported and how to get the data you actually want.

@ericpearson ericpearson closed this Nov 5, 2012
@ericpearson ericpearson reopened this Nov 5, 2012
@ericpearson
Copy link
Author

This is done refactoring and waiting for testing/code review, docs are forthcoming

@ericpearson
Copy link
Author

Docs are updated, please test and code review

@ericpearson
Copy link
Author

@rwmtse can I get a code review for this when you get a chance? @tracyli please test :)


Sensors::Sensors(const std::string& id) : m_id(id)
{
m_pSensorsController = new webworks::SensorsNDK(this);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this ever get deleted?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch, I will fix it

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now handled by destructor, should free everything now

@rwmtse
Copy link

rwmtse commented Nov 7, 2012

r+

@tracyli
Copy link

tracyli commented Nov 7, 2012

tested with packager build http://ci0000003863287:8080/hudson/job/BB10-Webworks-Packager/99/ and framework build http://ci0000003863287:8080/hudson/job/BB10-Webworks-Framework-next-sensors/34/: sensor events work as expected.
However, there's an observation:
1.automated function test widget for sensor got failed when run it at the first time; but when re-running the widget, it got passed.

@nukulb
Copy link

nukulb commented Nov 8, 2012

@tracyli has this been tested on the simulator?

@tracyli
Copy link

tracyli commented Nov 8, 2012

not yet; as I said yesterday the simulator 1035 shows up as black screen for me and I'll try again.

@ericpearson
Copy link
Author

this is impossible to test on the simulator

@nukulb
Copy link

nukulb commented Nov 8, 2012

@Pagey why?

@ericpearson
Copy link
Author

there is no way to compile for it as there isn't a library for sensors, plus the simulator has no way of emulating them

@nukulb nukulb closed this Nov 8, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants