-
Notifications
You must be signed in to change notification settings - Fork 54
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 I2C implementation #52
Add I2C implementation #52
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thankyou for creating your first PR! Contributions like yours is what it is all about. Keep it up!
@asymingt hey hey this is awesome stuff! I try to carve some time out soon to review this and test the UART stuff. So cool btw to hear this is working on a google coral board - haven't gotten one myself but they definitely look cool! |
@flynneva, glad you appreciate the contribution. The code does work, but it's not particularly high-performance. I'm seeing ~60Hz throughput when the node is run without anything else:
Also, the magnetic field sensor is somehow now also working, although I think the scale factor is off. |
These results are actually comparable to a pure C++ implementation (https://github.com/bdholt1/ros2_bno055_sensor) so I suspect this might have to do with (1) bus speed limitations, or (2) IPC overhead. I'm leaning towards the former. |
Worked out that we are bus limited -- it's either the i2c bus or the chip that is holding back the sampling frequency. Knowing this, I can get ~100Hz raw IMU and ~20Hz magnetometer with this PR, where I've separated our the publication into smaller i2c transactions. But I have to disable quaternion and gravitational vector publication: # For streaming raw inertial data.
Node(
package='bno055',
executable='bno055',
name='bno055_imu_node',
parameters=[
{ "ros_topic_prefix": "bno055/" },
{ "connection_type": "i2c" },
{ "i2c_bus": 0 },
{ "i2c_addr": 0x28 },
{ "imu_query_frequency": 100 },
{ "rot_query_frequency": 0 },
{ "gra_query_frequency": 0 },
{ "mag_query_frequency": 20 },
{ "tmp_query_frequency": 1 },
{ "cal_query_frequency": 1 },
{ "frame_id": "bno055" },
{ "operation_mode": 0x0C },
{ "placement_axis_remap": "P2" },
{ "acc_factor": 100.0 },
{ "mag_factor": 16000000.0 },
{ "gyr_factor": 900.0 },
{ "set_offsets" : False },
{ "offset_acc": [0xFFEC, 0x00A5, 0xFFE8] },
{ "offset_mag": [0xFFB4, 0xFE9E, 0x027D] },
{ "offset_gyr": [0x0002, 0xFFFF, 0xFFFF] }
],
output='screen'
), Here are my results on a Coral Dev Mini Board:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@asymingt overall awesome PR!!!! Love the work you did here and other than a few small comments everything looks great!
@asymingt this might be something we can just add to the docs later so others know this as well for I2C. If you want to add it to this PR feel free, but also we can just add it in another PR later as well...not a big deal either way. |
@flynneva I think I addressed your review comments. I added a couple of copyright headers to files that didn't have them. Hopefully that's OK. |
@flynneva feel free to merge this when you're ready to add the changes. |
@asymingt yes sorry! I am traveling for work this week (at CES) so I won't get to it this week but will make it a priority next week. |
@asymingt again, thank you for the contribution! 🙏🏼 stuff like this is what makes the ROS / OSS community so great 👏🏼 |
|
||
try: | ||
self.serialConnection.write(buf_out) | ||
buf_in = bytearray(self.serialConnection.read()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line should be buf_in = bytearray(self.serialConnection.read(2))
since the UART protocol register write answers with two bytes (preferably 0xEE 0X01 - Write success) which is tested later in line 140. Additional reasoning: previous implementation in bno055/connectors/Connector.py line 126 and datasheet page 93.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This modifies the code to add I2C support for the BNO055 sensor using the
smbus
library for I2C access. It addsi2c_bus
andi2c_addr
parameters to define the bus and address for the chip.TODO list:
ZeroDevisionError
on startupTested on a Google Coral Dev Mini board.
I do get two warnings about the first few readings being zero after initial configuration.
IMU data looks to not be garbage: