ceva-dsp / sh2

SH2 interface library for Hillcrest Laboratories, Inc BNO08x and FSP200 products.
41 stars 8 forks source link

Library cannot support more than one sensor #9

Open rah2501 opened 11 months ago

rah2501 commented 11 months ago

I'm using the SparkFun VR IMU BNO08X Arduino Library which imports this sh2 library. Unfortunately, the API in the BNO08x library cannot support more than one sensor as it uses global variables for sensor-specific data. I've been updating the library to enable its use with more than one sensor.

Unfortunately, I've come across a similar issue with your sh2 library. The BNO08x library calls sh2_service() which uses another global variable named _sh2 for sensor-specific state.

rah2501 commented 11 months ago

Can I ask if there is a specific reason you used a global variable to store sensor state? Are you open to changing the API so that the library can be used with more than one sensor?

rah2501 commented 11 months ago

I've opened #10 as an example of how the API might be changed to support more than one sensor. What are your thoughts?

rah2501 commented 10 months ago

Hello? @kay-ceva ?

WolfgangWindholtz commented 9 months ago

@rah2501 Have you figured this issue this issue out. We are trying figure out the same issue for a project.

rah2501 commented 9 months ago

@rah2501 Have you figured this issue this issue out.

No. I don't want to waste time making changes to the library that might not be accepted by upstream. I'm still waiting on the maintainers (Ceva) to respond. It looks like they might not be interested in collaborative development.

@dwheeler-ceva ?

iLoveAndyBaker commented 2 weeks ago

If the upstream people are not going to make the changes, through no bad faith on their part of course, then why doesn't the community do it instead? It seems just as likely that the upstream folks won't make ANY new changes anyway.

rah2501 commented 2 weeks ago

why doesn't the community do it instead?

Because the community doesn't have write privileges to the repository.

iLoveAndyBaker commented 2 weeks ago

Rather I should say, leave the upstream version well enough alone, and pack a "fixed" version in the Sparkfun library or whoever happens to be using it

dwheeler-ceva commented 2 weeks ago

The code in this repo was originally intended as an example implementation, not necessarily as a ready-to-use driver. We chose to support just one device because it was the simplest, most universal case. (For similar reasons, there's no OS integration here.)

I guess what I need to say here is you should consider forking this repo and working together to implement the changes you need.