Address initial comments on updates to firmware #19
Conversation
bparks13
commented
Feb 21, 2025
bparks13
commented
- Find device address for methods based on the given port number
- Reorder parameters to be consistent with oni_* calls
- Before reading device IDs from an EEPROM, confirm that the first four bytes contain the characters "OESH"
- Make readBytes() more flexible, and create readEepromBytes() specifically for EEPROM byte reads
- Corrected logic for devices with a BNO but no EEPROM
- Reorder parameters to be consistent with oni_* calls - Before reading device IDs from an EEPROM, confirm that the first four bytes contain the characters "OESH" - Make readBytes() more flexible, and create readEepromBytes() specifically for EEPROM byte reads - Corrected logic for devices with a BNO but no EEPROM
| { | ||
| evalBoard->setBnoAxisMap (i, 0b00100100); | ||
| } | ||
| else if (hasI2c[i]) |
There was a problem hiding this comment.
Since we are using this check to set BNO axis maps, I'd use hasBNO instead of hasI2C here.
| @@ -1204,43 +1206,80 @@ bool Rhd2000ONIBoard::isI2cCapable(oni_dev_idx_t device) | |||
| return val > 0; | |||
| } | |||
|
|
|||
| int Rhd2000ONIBoard::readByte(uint32_t address, oni_reg_addr_t i2cRawAddress, oni_reg_val_t* value, bool sixteenBitAddress) | |||
| int Rhd2000ONIBoard::readByte (oni_dev_idx_t device, uint32_t address, oni_reg_val_t* value) | |||
There was a problem hiding this comment.
This is not what I meant. This is basically just a wrapper over oni_read_reg at this point with nothing added to it.
What I meant is int Rhd2000ONIBoard::readI2CByte(oni_dev_idx_t device, uint32_t i2cDevAddress, oni_reg_addr_t i2cRegAddress, oni_reg_val_t* value, bool sixteenBitAddress)
Just not have the eeprom harcoded here as it was before, but make this a general function to read bytes over i2c
then on readEepromByte you call this method with i2cDevAddress = 0x50 and sixteenBitAddress = true
| oni_dev_idx_t i2cRawAddress = DEVICE_I2C_RAW_A + port * 2; | ||
|
|
||
| // NB: Confirm EEPROM first four bytes contains OESH | ||
| const char identifier[] = "OESH"; |
There was a problem hiding this comment.
just to be sure, make it "OESH\x01" to include the spec version, in case we change it later, so even an outdated plugin would not break.
