-
Notifications
You must be signed in to change notification settings - Fork 8
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
Accelerometer and Gyroscope support on mobile devices #50
Conversation
…reflect path changes.
…ents via button press by user on ios devices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! If you could fix the enableGyro/Accel
stuff that'd be great! Others are just some small documentation/nitpicks ;)
Thanks for making these two classes happen Mike!
src/gyro/Gyro.ts
Outdated
|
||
|
||
/** | ||
* Introducing Gyro (gyroerometer, on mobile) support for WebChucK. Gyro wraps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Do you mean gyrometer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yes, gyroscope*
src/gyro/Gyro.ts
Outdated
* iOS devices require that the web developer ask permission from the user to access sensors after a button push. This looks like: | ||
* | ||
* ```ts | ||
* import { Chuck, Gyro} from '../webchuck/src/wc-bundle.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When mentioning importing for documentation, you can just say import { Chuck, Gyro } from "webchuck"
. The location is different on every platform so it's easier to keep it general.
Nit: Can you make sure there's a opening and closing space between { Chuck, Gyro }
src/gyro/Gyro.ts
Outdated
/** | ||
* Initialize Gyro functionality in your WebChucK instance. | ||
* This adds a `Gyro` and `GyroMsg` class to the ChucK Virtual Machine (VM). | ||
* Gyroerometer event (DeviceMotionEvent) listeners are added if `enableGyro` is true (default). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Gyrometer
src/gyro/Gyro.ts
Outdated
* ``` | ||
*/ | ||
enableGyro() { | ||
// consider using "deviceorientationabsolute" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we want to still consider this functionality down the line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so. It isn't supported on some browsers, and seems deviceorientation is the more supported variety.
return deviceName; | ||
} | ||
|
||
function int openGyro(int num) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just thinking out loud for the future here, no changes requested, but perhaps in the future, we should actually check if the gyro is working via js lol :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that would be smart. Still some things to sort or keep an eye on there.
@@ -1,7 +1,7 @@ | |||
//======================================================================= | |||
// WebChucK Test Suite | |||
//======================================================================= | |||
import { Chuck, HID } from '../src/wc-bundle.js'; | |||
import { Chuck, HID, Gyro, Accel} from '../src/wc-bundle.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: import bracket spacing
if (num < 0) { | ||
false => active; | ||
} else { | ||
"js DeviceMotionEvent" => deviceName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Is the device name js DeviceMotionEvent
or more like js DeviceMotionSensor
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there is a specific device name... it's just representing access to device orientation and acceleration. I made up this shorthand reference... we could just call it "accelerometer" or "gyroscope" as that is essentially what we are accessing. What do you think?
src/accel/Accel.ts
Outdated
await accel.theChuck.runCode(Accel_ck); | ||
|
||
// Enable mouse and keyboard | ||
accel.enableAccel(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line of code should only be run when enableAccel
is true.
src/gyro/Gyro.ts
Outdated
await gyro.theChuck.runCode(Gyro_ck); | ||
|
||
// Enable mouse and keyboard | ||
gyro.enableGyro(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line of code should only be run when enableGyro
is true.
src/accel/Accel.ts
Outdated
* iOS devices require that the web developer ask permission from the user to access sensors after a button push. For example: | ||
* | ||
* ```ts | ||
* import { Chuck, Accel} from '../webchuck/src/wc-bundle.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: import bracket spacing and can just make it import { Chuck, Accel } from "webchuck"
like above
LGTM! |
Accel
andGyro
classes implemented for WebChucK to be used on mobile devices. Both modeled afterHID
,Accel
and
Gyro
interface with browser/JS "devicemotion" and "deviceorientation" events, respectively, and allow you to listen for and receiveAccelMsg
's orGyroMsg
's inline with WebChucK code, providing x, y, z data in each case.Big thanks to @terryzfeng for assistance in the architecture here!