-
Notifications
You must be signed in to change notification settings - Fork 130
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
[driver] Adding InvenSense 6-Axis IMU driver #1040
Conversation
Thanks for the driver!
You can add the compatible devices into the module description then it is also findable via the homepage.
I'll review in detail before the end of the week this still gets into the 23Q2 release. |
Thanks for the nice comments! I have addressed point 1, 2 and 3 in the latest commits. I plan to address point 4 tomorrow with my first suggested design. Also I was reading the documentation for ICM-42688-P in greater detail and found that some registers are not identically the same as for IIM-42652. For example the APEX_CONFIG0 is a little different between the two devices. Therefore I have not added an admonition that the driver is compatible with e.g. ICM-42688-P. Is this still relevant or should I leave it as is? |
I have started the FIFO implementation and have had some problems with the endianess of the imu which I will have to look into. Specifically, I configure the device to use little endian for both sensor data and fifo count. However, setting the fifo count to use little endian, seem to have the oppostive effect. Every time that I have configured the imu to use little endian and have used (FIFO_COUNTH << 8) | FIFO_COUNTL get a byte count in excess of 4000. That is despite the FIFO only having 2K bytes. I will have to look into this tomorrow. |
I have had some trouble getting the FIFO to work. The trouble was caused by setting the FIFO_WM_GT_TH bit in FIFO_CONFIG1 or setting TMST_EN in TMST_CONFIG. Altough I would not have expected setting these bits high would have caused the FIFO_DATA register to contain zero readings. However, currently I am getting readouts through FIFO so now I only need to parse the data, which I have already started. |
I have been a bit busy the last week and hence the slow progress. I have now implmented a parser for parsing the fifo data and an iterator which uses the parser to iterate through the fifo data. I will now have to test that the iterator works as intended. Also I will have to figure out if there is a nice way to pass the scale onto the FifoPacket class or if this should be up to the user to provide the scale. |
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.
Very thorough driver, impressive! Just some minor nitpicks from me.
Thanks for the nice comments! It has taken a lot of energy getting the driver to this state. I choose to rename the driver as in this repo. I believe this is fine? I might ad an admonition that the driver is based on iim42562 and that some registers might be missing or not applicable to other InvenSense devices? What is missing now is to somehow pass the accel and gyro scale onto the FifoPacket or in another way implement code that can convert the raw readings to some scale with physical meaning. The obvious choice is to have FifoData pass it onto the iterator which passes it onto the FifoPacket. However, I think that is a bit clumpsy. Do you have any suggestions? In addition, I have yet to implement support for APEX data. However, this outside of my personal use case, so I prefer to add it at later. Finally, I was debating if I should remove the ixm42xxx.transport module and let the ixm42xxx module copy the transport files? I believe the intention behind lis3.transport was to make it available to multiple drivers for example lis3dsh and lis302dl, which have different implementations. |
The renaming is fine, especially with documentation about possible limitations and tested versions. You can remove the separate transport module, we can add it back if needed, just leave the classes in separate files. I'm not sure about the how to pass the scale, I don't think you have much choice unless you convert it before passing it to the packet. |
I now consider this ready for merge unless you have any additional changes? Before merging I suppose that I will change up the git history to have: [math] Adding long vector typedefs I have choosen to implement the FifoPacket scaling methods by passing the scale to the method. This servers two purposes. First of all I avoid having to pass the scales twice. More importantly the scales are stored only in one place, so there are not multiple sources of truth. What do you prefer? Also I should add that the when compiling I get -Wdouble-promotion warning from the printf statements. I suppose that I should simply ignore this? |
If you pass parameters to a C-style variadic function like |
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.
Excellent driver! Please rebase/squash as you described.
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.
Thank you!
5e4bda8
to
32ad5d1
Compare
32ad5d1
to
8012d82
Compare
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.
Thanks!
I have created this driver for IIM-42652, but it also works for ICM-42688-P and possibly many other InvenSense IMUs. The only difference between IIM-42652 and ICM-42688-P, that I have noticed from a software perspective is that IIM-42652 has Register Bank 3. Therefore I do not know if Iim42652 is the right name for the device?
In addition I have a couple of points that I would like your take on:
I have some different design ideas for the FIFO interface that I will like your view.
In all circumstances I will add an iterator to the class that handles FIFO data for parsing the data and return parsed FIFO frames. I hope my considerations make sense and thanks in advance!