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

Add Per-Sensor Descriptor and Support Multiple IMUs #249

Merged
merged 17 commits into from
Jul 28, 2023

Conversation

nekomona
Copy link
Contributor

@nekomona nekomona commented Jun 3, 2023

This modification introduces sensor descriptor to specify sensor config in a clearer way. Currently it contains sensor types, address, pins and extra configs like axis remapping. It may extend to more parameters like magnetometer type in the future.

Descriptors are written in macros in the following format thanks to @yoyobuae :

#define IMU_DESC_LIST \
             IMU_DESC_ENTRY(IMU_MPU6050, 0x68, DEG_270, PIN_IMU_SCL, PIN_IMU_SDA) \
             IMU_DESC_ENTRY(IMU_BMI160,  0x69, DEG_270, PIN_IMU_SCL, PIN_IMU_SDA, AXIS_REMAP_DEFAULT)

They will be expanded to sensor building and detecting in the SensorManager. Sensor ID is assigned by the sequence of sensors in the list, and other parameters will be passed to building functions. This creates a direct connection between sensor id, sensor address, and sensor pins / configs in the same place.

The new descriptor also allows to add more than 2 sensors by appending more lines to the descriptor. To implement this, each sensor config contains the pin for that sensor. Internal pinmux inside ESPs are used to switch between different sensors, so more than 2 sensors with the same base address could be hooked up to different pins without extra hardware.

The code is highly experimental but attempted to be compatible with previous configs. When not specified, the descriptors will be generated from previous defines with matching address and ids. If a previous config uses two types of sensors but with incorrect address pin connection, the new code won't try the other address to avoid mismatch between config and assigned sensor id.

Axis remapping for BMI160 may not be fully compatible with previous configs. Now axis mapping is described by a number generated from defines. The sensor will use the number for no remapping by default to match the previous default behavior. Each BMI160 could have its own number for a different remapping in its descriptor. To migrate for this patch, previous changes in bmi160_defines.h for axis mapping will need to be rewritten in the new descriptors unless no remapping is used.

I have tested this in 2-sensor setups with combination of BMI160 and MPU6050s. An experimental setup driving 8 MPU6050s are also tested and works with only additional lines in the descriptor. INT pin for BNOs may need to be tested but should be working as the only difference is in passing pin numbers.

Copy link
Member

@TheDevMinerTV TheDevMinerTV left a comment

Choose a reason for hiding this comment

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

LGTM, I've still added some comments though

Copy link
Member

@Eirenliel Eirenliel left a comment

Choose a reason for hiding this comment

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

Another banger. Really want to merge this and the other one, and also merge my refactoring so there are less conflicts lol

src/serial/serialcommands.cpp Outdated Show resolved Hide resolved
src/serial/serialcommands.cpp Outdated Show resolved Hide resolved
@Eirenliel
Copy link
Member

Firmware webflasher would need to be rebuilt for this... @ButterscotchV @loucass003...

@nekomona nekomona closed this Jul 16, 2023
@nekomona nekomona reopened this Jul 16, 2023
@Eirenliel Eirenliel merged commit 228a2dd into SlimeVR:main Jul 28, 2023
pinMode(activeSDA, INPUT);

if (running) {
i2c_set_pin(I2C_NUM_0, sdaPin, sclPin, false, false, I2C_MODE_MASTER);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason to deactivate the pull ups for SDA/SCL?

@nekomona nekomona deleted the multiimu-strdesc branch March 16, 2024 12:04
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