search

TheThingsIndustries / generic-node-se

Generic Node Sensor Edition
https://www.genericnode.com
Other
109 stars 31 forks source link

Feature/freefall app #120

Closed mcserved closed 3 years ago

mcserved commented 3 years ago

Summary:

A basic implementation of the Freefall App. The node connects over OTAA and waits for free fall events. If they occur, then the amount of free falls is added to a payload and transmitted. Closes #113.

Changes:

Notes for Reviewers:

The default parameters are based on this: https://www.google.com/url?sa=t&rct=j&q=&esrc=s&source=web&cd=&ved=2ahUKEwjrl_ftvevuAhVQzYUKHVFlBDUQFjAAegQIBRAC&url=https%3A%2F%2Fwww.st.com%2Fresource%2Fen%2Fdesign_tip%2Fdm00500824-setting-up-freefall-recognition-with-sts-mems-accelerometers-stmicroelectronics.pdf&usg=AOvVaw2EvAql8RUKdAtUcV6iYKOK This amount seems about right, where free falls are detected but rebounds from bouncing on the surface are not. I tested by dropping it on my bed from arms length (yanking it up just right on my desks also works). The second transmissions usually needs around 3 min for the next transmission, you can try dropping it a few times to see if it counts the amount you dropped it.

Release Notes: (optional)

...

mcserved commented 3 years ago

@elsalahy I've fixed all comments and fixed some other minor issues as well.

elsalahy commented 3 years ago

@marnixcrouzen please re-request a review, otherwise we will not review

mcserved commented 3 years ago

One minor detail, .gitignore has this line:

/Software/app/basic*/Debug/

for STM32CubeIDE build paths. This does not work for this specific case because its named freefall_lorawan. The same affects freertos_lorawan. Should I change the name to basic_freefall, leave it or change .gitignore to:

/Software/app/*/Debug/

mcserved commented 3 years ago

Another note, should I update the README to include this app?

elsalahy commented 3 years ago

One minor detail, .gitignore has this line:

/Software/app/basic*/Debug/

for STM32CubeIDE build paths. This does not work for this specific case because its named freefall_lorawan. The same affects freertos_lorawan. Should I change the name to basic_freefall, leave it or change .gitignore to:

/Software/app/*/Debug/

@marnixcrouzen the app name is ok, freefall_lorawan is good, please just add your application debug path in the gitignore, and this doesn't apply to freertos_lorawan because it doesn't support STM32Cube IDE yet.

elsalahy commented 3 years ago

@marnixcrouzen also please rebase and handle needed changes, such as changes in conf, and lora_app handlers

elsalahy commented 3 years ago

The commits look good but as a I mentioned, you need to rebase this over develop branch, as this would not compile once you rebase.

@marnixcrouzen is my comment unclear?

mcserved commented 3 years ago

The commits look good but as a I mentioned, you need to rebase this over develop branch, as this would not compile once you rebase.

@marnixcrouzen is my comment unclear?

@elsalahy My history looks like this now, this is not correct?

I thought you meant rebasing from develop, what should I do then?

elsalahy commented 3 years ago

@marnixcrouzen your Git history is correct, but the application dependancies and the stack changed when you rebased, so now your freefall application doesn't compile, try a clean build and you will observe the issue.