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

Implemented steps reset function for bma421 #1

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

Conversation

BNolet
Copy link

@BNolet BNolet commented Dec 24, 2020

  • added (useless) param to make compatible with bma42x.c
  • implemented reset_step_counter in bma421.c/h

* added (useless) param to make compatible with bma42x.c
* implemented reset_step_counter in bma421.c/h
BNolet pushed a commit to BNolet/wasp-os that referenced this pull request Dec 24, 2020
**This commit relies on the PR wasp-os/bma42x-upy#1

* Implement touch action in pedometer app
* Impelement more efficient step counter reset
BNolet pushed a commit to BNolet/wasp-os that referenced this pull request Dec 24, 2020
**This commit relies on the PR wasp-os/bma42x-upy#1

* Implement touch action in pedometer app
* Impelement more efficient step counter reset

Signed-off-by: Brandon Nolet <linuxliaison@fastmail.com>
@BNolet
Copy link
Author

BNolet commented Dec 24, 2020

Just a little backstory regarding this commit.

I originally figured it would be as simple as using the existing reset function from steps.py pedometer app in wasp-os but that ended up taking quite a long time (on the magnitude of several seconds) every time I tapped the app to reset the pedometer to 0. So I dug and then I found the reset function was actually meant to be more efficient.

Once I found the source of the issue, it was simply a matter of implementing the proper reset function. The issue is 1. I know very little C, and 2. The setter function in bma42x.c was not immediately compatible with the reset function of bma421.c. So I had to add some useless param to the library.

I'm sure there's a less hacky way to do this but I'm sure that can be figured out by someone with more C competence than I.

Copy link
Collaborator

@daniel-thompson daniel-thompson left a comment

Choose a reason for hiding this comment

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

I'm afraid you are right that adding a useless argument simply to allow you to treat bma421_reset_step_counter() like a setter method is hacky.

bma421_reset_step_counter() is not a setter function so you should not wrap it like a setter. It is an action (like init or write_config_file) and should be wrapped in the same way as init.

See bma42x_BMA42X_init and BMA4_EXPORT_OBJ(init) for examples of the correct code to copy and note that the wrapping code must use the same name as the underlying C API (e.g. the function that should end up in the locals dict table should be BMA4_EXPORT_OBJ(reset_step_counter) ).

* Remove setter wrapper
* Add proper wrapper for reset_step_counter
@BNolet
Copy link
Author

BNolet commented Dec 26, 2020

Thank you Daniel for the guidance on this one as well as the proper explanation. I really appreciate being able to contribute to the project :)

I've made the requested change after testing that the result is the same with my PineTime

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.

2 participants