search

ftctechnh / ftc_app

FTC Android Studio project to create FTC Robot Controller app.
761 stars 3.16k forks source link

Need APIs to enable/disable any I2C devices for improving sensor performance. #296

Open trc492 opened 7 years ago

trc492 commented 7 years ago

Since my previous ticket on gyro performance issue has been closed because of the release of beta 2.61, I just want to open a separate ticket on the rest of the sensor performance issues so they don't fall through the cracks. The details of the issue can be found on this forum thread: http://ftcforum.usfirst.org/showthread.php?7724-Gyro-Read-Latency-Development-team-update

cmacfarl commented 7 years ago

This is largely an unavoidable aspect of the hardware architecture of the system and while there may be some areas of small improvement here, we don't have a CAN bus, and likely never will. :-)

If you have a specific problem and have data showing what that problem is, then please post it here. However that forum thread is rather long and discusses many different aspects of the problem and isn't the best core reference for a bug report itself.

cmacfarl commented 7 years ago

To elaborate on this further, the CDIM is somewhat special as it has two distinctly different types of device ports. 1) Analog/Digital IO ports which don't need to be queried to determine their state, and 2) I2C devices which do.

The portion of the memory map which contains the first is read on every iteration through the usb runnable for that device.

Profiling shows that each read of any usb device takes roughly 10ms. The amount of data read, for FTC purposes, has little to no impact on the transaction time. USB is optimized for bulk data. And we are using it in a distinctly different application.

The portion of the memory map which contains the second is read for each I2C port which has requested a read. Which means that if you have no I2C devices attached to the CDIM you'll incur the overhead of a single read of the A/D portion of the memory each iteration through the runnable, but each I2C device incurs an additional read, along with an addition write to tell the CDIM port to execute the I2C transaction, and all the overhead that is associated with USB transactions.

Add in contention for the bus from N other controllers for which there is no optimization as all the other controllers read the entire memory map and you see where we are today.

All this said, in practice, a 50ms latency for a motor controller isn't typically a match losing problem. Servo controllers will typically get told to move a servo to a known position and so will also perform acceptably. Which means we are left with I2C sensors attached to a CDIM for which perhaps there may be a slight optimization from a read of the entire memory map and then a byte copy for those ports that are requesting reads, but for which it's undetermined at this point in time whether or not this is something we want to do mid-season.

Unlike the gyro problem, which was clearly necessary.

On Tue, Dec 20, 2016 at 2:59 PM, Craig MacFarlane notifications@github.com wrote:

This is largely an unavoidable aspect of the hardware architecture of the system and while there may be some areas of small improvement here, we don't have a CAN bus, and likely never will. :-)

If you have a specific problem and have data showing what that problem is, then please post it here. However that forum thread is rather long and discusses many different aspects of the problem and isn't the best core reference for a bug report itself.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ftctechnh/ftc_app/issues/296#issuecomment-268384285, or mute the thread https://github.com/notifications/unsubscribe-auth/ABXn94_s5bZCfAJ0KgPczwLhs_Or1oLcks5rKF3XgaJpZM4LSX8z .

trc492 commented 7 years ago

Phil's first posting mentioned about 3 areas that can be fixed. Beta release 2.61 only fixed the first area. So it gave me an impression more fixes will be coming. Where I used to work, we have to have an open bug in order to make a fix. If there is no open issue, there will be no fix. So I thought I should open an issue to keep it alive. I think we should at least fix the 2nd area (adding enable/disable of any I2C devices). Sorry about not being specific on the bug title. I have changed the title to reflect the real ask. Please note that I am not asking this to be fixed in this season. Just want to keep this in the list of things to be improved.