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

Glove #371

Open
wants to merge 37 commits into
base: main
Choose a base branch
from
Open

Glove #371

wants to merge 37 commits into from

Conversation

Eirenliel
Copy link
Member

@Eirenliel Eirenliel commented Dec 7, 2024

Pretty complex PR that changes how defines.h works, so it needs changes from all firmware tools @ButterscotchV @loucass003

Changes

  • Abstract hardware interface for communicating with IMUs: can now easily add support for multiple pinouts, muxers, shifters, extenders, etc. by just adding a class file
    • Interfaces currently implemented: direct pin i2c, direct int pin, PCA9546A I2C expander, MCP23X17 GPIO expander
    • Sensor defines changed to SENSOR_DESC_LIST and SENSOR_DESC_ENTRY, and type of hardware interface is required. DIRECT_WIRE for directly connected I2C, DIRECT_PIN for directly connected INT. This is the breaking change for firmware tools.
  • Send sensor position information (joint the seonsor controls, not 3d position) and sensor data type as defined in flex and glove PR's of the server
    • Position is set via new defines SENSOR_INFO_LIST and SENSOR_INFO_ENTRY, see examples in defines.h
  • Reports and saves BNO085 calibration data. It seems to be pretty useless, but it's a bit of information...
  • Disables BNO085 accel calibration 1 minute after startup. Was hoping to fix stomping issue with it, but alas...

Defines changes

  1. Maximum sensors count
- #define MAX_IMU_COUNT 2
+ #define MAX_SENSORS_COUNT 2
  1. IMU list is now sensor list
- #define IMU_DESC_LIST \
+ #define SENSOR_DESC_LIST \
- IMU_DESC_ENTRY(...)
- SENSOR_DESC_ENTRY(...)
  1. Sensor pinouts/hardware interface are now need to be wrapped in helpers:
- IMU_DESC_ENTRY(IMU,  PRIMARY_IMU_ADDRESS_ONE, IMU_ROTATION, PIN_IMU_SCL, PIN_IMU_SDA, PRIMARY_IMU_OPTIONAL, PIN_IMU_INT) 
+ SENSOR_DESC_ENTRY(IMU, PRIMARY_IMU_ADDRESS_ONE, IMU_ROTATION, DIRECT_WIRE(PIN_IMU_SCL, PIN_IMU_SDA), PRIMARY_IMU_OPTIONAL, DIRECT_PIN(PIN_IMU_INT), 0)
  1. Following hardware interfaces supported:
  • DIRECT_WIRE(scl, sda) - I2C on GPIO
  • PCA_WIRE(scl, sda, pca_addr, pca_channel) - I2C on PCA9546A I2C expander
  • DIRECT_PIN(pin) - INT pin on GPIO
  • MCP_PIN(pin) - INT pin on MCP23X17 GPIO extender
  • NO_PIN - INT pin not used
  1. Added SENSOR_INFO_LIST and SENSOR_INFO_ENTRY where currently a default sensor position can be defined

Test plan

There are a lot of changes, so we should do some testing before it's merged. I did some, but we need help with testing with other IMUs

  • Test with BNO085 glove + ESP32-C3 MCU + PCA9546A I2C extender + MCP23X17 GPIO extender
  • Test with official BNO085 slime and ESP12-F
  • Test with official BNO085 slime + official BNO085 extension
  • Test ESP32 with any IMU
  • Test ESP32-C3 with any other IMU
  • Test ESP12-F with any other IMU
  • Test with BMI160
  • Test with BMI270
  • Test with LSM6DSV
  • Test with ICM-42688
  • Test with different IMU as extension compared to main
  • Test with more than 2 IMUs on different I2C pins

I'm once again asking for help with testing!

Other important stuff:

  • Prepare firmware tools for this change for FW 0.6.0+
  • Bump firmware and protocol versions

WHY, EIREN??

Now we can do THIS in mainline firmware!
IMG20241207135122

Future stuff

Currently with 10 BNO sensors I get only 10 TPS on all of them, even when not all of them move. I think it can be optimized, and maybe we should start using additional cores where available. But that's out of scope right now, the goal was to support more sensors, not to make them work amazingly.

@Eirenliel Eirenliel requested a review from ImUrX December 7, 2024 13:47
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.

Please rebase to main and also move all of the unnecessary formatting commits into a different PR. It's impossible to review this PR otherwise.

Comment on lines +30 to +33
virtual void reset();
virtual void update();
virtual float getAveragedTPS();
virtual float getTPS();
Copy link
Member

@TheDevMinerTV TheDevMinerTV Dec 29, 2024

Choose a reason for hiding this comment

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

These don't need virtual since they're not overridden by anything.

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