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

fix #3 #11

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

fix #3 #11

wants to merge 11 commits into from

Conversation

tkhabia
Copy link

@tkhabia tkhabia commented Mar 7, 2020

added AccelerometerTap code.

added AccelerometerTap code.
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

You have forgotten to account for pseudo-acceleration caused by gravity. This causes whichever axis is pointing down to constantly register taps.


Please do a Tools > Auto Format if using the Arduino IDE or Ctrl + b if using Arduino Web Editor so that the formatting of your code will be consistent with Arduino's other examples.

examples/AccelerometerTap/AccelerometerTap.ino Outdated Show resolved Hide resolved
examples/AccelerometerTap/AccelerometerTap.ino Outdated Show resolved Hide resolved
examples/AccelerometerTap/AccelerometerTap.ino Outdated Show resolved Hide resolved
examples/AccelerometerTap/AccelerometerTap.ino Outdated Show resolved Hide resolved
Comment on lines 14 to 20

while (!Serial);

while (!IMU.begin()) {

Serial.println("Failed to initialize IMU!");

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
while (!Serial);
while (!IMU.begin()) {
Serial.println("Failed to initialize IMU!");
while (!Serial);
while (!IMU.begin()) {
Serial.println("Failed to initialize IMU!");

Remove pointless blank lines.

examples/AccelerometerTap/AccelerometerTap.ino Outdated Show resolved Hide resolved
examples/AccelerometerTap/AccelerometerTap.ino Outdated Show resolved Hide resolved

}

float tapThreshold = 0.05 ; //0.05g acceleration in some direction is considered as tap. it can be change for the required sensitivity.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
float tapThreshold = 0.05 ; //0.05g acceleration in some direction is considered as tap. it can be change for the required sensitivity.
float tapThreshold = 0.05; //0.05 g acceleration in some direction is considered as tap. it can be changed for the required sensitivity.


float tapThreshold = 0.05 ; //0.05g acceleration in some direction is considered as tap. it can be change for the required sensitivity.

int down = 3 ; // signifing the direction of which is facing downward 1 for x axis ; 2 for y axis ; 3 for z axis;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
int down = 3 ; // signifing the direction of which is facing downward 1 for x axis ; 2 for y axis ; 3 for z axis;
int down = 3; // signifying the direction which is facing downward: 1 for x axis; 2 for y axis; 3 for z axis

Fix formatting and typo.


Why not make this a char so you can do this:

char down = 'z'; // the axis which is facing downward


Serial.println("Tap detected across X-axis");
}
if ((y > tapThreshold || y < -tapThreshold) && down != 2 ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if ((y > tapThreshold || y < -tapThreshold) && down != 2 ) {
if ((y > tapThreshold || y < -tapThreshold) && down != 2) {

Fix formatting.


Serial.println("Tap detected across Y-axis");
}
if ((z > tapThreshold || z < -tapThreshold)&& down != 3 ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if ((z > tapThreshold || z < -tapThreshold)&& down != 3 ) {
if ((z > tapThreshold || z < -tapThreshold) && down != 3) {

Fix formatting.

examples/AccelerometerTap/AccelerometerTap.ino Outdated Show resolved Hide resolved
Arduino LSM6DS3 - Accelerometer Tap

this code is to detect tap

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Remove pointless blank lines.

@tkhabia
Copy link
Author

tkhabia commented Mar 21, 2020

do you wish for any other changes?

Copy link

@MisterAwesome23 MisterAwesome23 left a comment

Choose a reason for hiding this comment

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

Hey @tkhabia
Thanks for the pull request :)

I have a few queries with the suggested code-

  1. How can we detect a tap along all directions? Setting down to a value of 3 (hard-coded) and with this logic, we will not be able to leverage of one of the directions Z axes in this case.

  2. I agree with @per1234 of using char for X,Y,Z for direction rather than 1,2,3 since it is more intuitive.

  3. Consider adding float tapThreshold and int down in void setup(){} since they're a part of initialization and makes more relevance there.
    More on this- https://forum.arduino.cc/index.php?topic=203395.0

  4. If LSM6DS3 provides with positive and negative axes (Not sure on this, CurieIMU does, maybe this will help- https://www.st.com/resource/en/datasheet/lsm6ds3.pdf) direction as well consider adding that as well since it'll be more accurate

tkhabia added 2 commits April 1, 2020 14:18
tried to resolve the requested querry.
@CLAassistant
Copy link

CLAassistant commented Apr 9, 2021

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants