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

CurieIMU scoping of methods makes sub-classing difficult. #176

Closed
apiascik opened this issue May 6, 2016 · 5 comments
Closed

CurieIMU scoping of methods makes sub-classing difficult. #176

apiascik opened this issue May 6, 2016 · 5 comments
Milestone

Comments

@apiascik
Copy link

apiascik commented May 6, 2016

I need more functionality from the BMI160 than the Curie board library makes available. I also prefer /require access to several methods from the BMI160 class that the CurieIMU class now wraps/hides.

I am requesting that the following functions be made protected to allow for CurieIMU subclassing:

uint8_t reg_read (uint8_t reg);
void reg_write(uint8_t reg, uint8_t data);
void reg_write_bits(uint8_t reg, uint8_t data, unsigned pos, unsigned len);
uint8_t reg_read_bits(uint8_t reg, unsigned pos, unsigned len);

int serial_buffer_transfer(uint8_t *buf, unsigned tx_cnt, unsigned rx_cnt);

In addition, I am requesting that any BMI160 class functions the CurieIMU class wraps/hides be made accessible (rename, etc.). Examples include:

float getFreefallDetectionThreshold();
...
void setDoubleTapDetectionDuration(int duration);

I'm sure some people find the CurieIMU wrapper useful. It would be nice if the direct access to the BMI160's registers (albeit incomplete) was still available for those who want it. I'd prefer to add on to the libraries you provide, not replace/Frankenstein them.

@eriknyquist
Copy link
Contributor

@SidLeung, can you see any issues with the above reg_* function delclarations being moved into "public" so users can access them?

@SidLeung
Copy link
Contributor

Understood the request for the accessing the BMI internals. Please be aware, from our POV, the arrangement/decision of wrapping and hiding the BMI class/info behind the CurieIMU class. In a future event, if it ever occurred, that the BMI160 is replaced, the significant changes would be limited within the boundary of the CurieIMU class. The existing Sketches that utilize the CurieIMU would not be impacted. It is not desirable for us to expose the BMI class/methods for this particular reason.

@eriknyquist
Copy link
Contributor

I agree with @SidLeung.

@PaulStoffregen
Copy link
Sponsor

PaulStoffregen commented Jun 27, 2016

It is not desirable for us to expose the BMI class/methods

How meaningful is CurieIMU's abstraction, really? Last time I looked, you're exposing quite a lot of BMI specific implementation, like the raw integers where you leave scaling to the end user, and you require the end user to know the specific ranges the BMI sensor implements. A long list of functions correspond 1:1 to BMI's specific motion processing features, which are similar but not identical to other chips on the market.

Look, I don't want to be too negative and cynical here. But if you really in your heart believe some measure of hardware independence is a good thing, perhaps you ought to take an honest objective look at the API you're currently publishing. It leaks a lot of BMI hardware specific details.

@apiascik
Copy link
Author

I'm not publishing an API. In my case, knowing the details of the hardware (BMI160) was critical to/preferable for the application. I was simply suggesting that it wouldn't take much effort to allow someone to bypass the HAL, especially if the intent was to play around with the Arduino101 they have in hand (like me). At the time I was working on this, the BMI160 datasheet was the best source of information on the Curie.

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

No branches or pull requests

5 participants