Battery: Add Tattu CAN driver #20824
Conversation
|
I guess this will work with any of the "Smart" Tattu batteries with an AS150U plug including this one. |
|
That’s what I tested with |
|
Why don't you do CRC checks on incoming messages? |
|
Which model #s of batteries does this work on? Need that for documentation so users don't select the wrong driver. |
|
This driver is hard-coded for the 12S flavor via |
yup, good point. It's being ignored for now, like all the other non-DroneCAN drivers. It should be easy to add, just need to call |
| } | ||
|
|
||
| // message complete! | ||
| memcpy((uint8_t*)&_message, data, _message_offset); |
There was a problem hiding this comment.
Why do we need this memcpy? Why not just have _message and data be a union, and then the only other things you need to store are just the cycle/remaining percentages, which are both currently being read without a semaphore anyway, and could be corrupt readings.
There was a problem hiding this comment.
reworked to use a union, good idea! I was keeping them separate because I was hoping to make a larger buffer to handle 12S and 14S but lets get this 12S one in and then add more features later.
| return false; | ||
| } | ||
|
|
||
| cycles = _message.cycle_life; |
There was a problem hiding this comment.
This should be protected by a semaphore if reading the backing array, or better still be reading from a copy aside that gets updated as part of read()
| return false; | ||
| } | ||
|
|
||
| percentage = _message.remaining_percent; |
There was a problem hiding this comment.
Same as below, use a copy aside during read rather then directly accessing the _message struct which is a memcpy target.
There was a problem hiding this comment.
This really just needs to be added to AP_BattMonitor::BattMonitor_State since UAVCAN also suffers from this. I can't do a semephore lock here because the backend is declares this function as const so even if it's added to the status (and therefore interim_state) we'll still need to change the backend
There was a problem hiding this comment.
If you have to deconst the function that's still better then the random interrupt. But on first glance I'm not opposed to moving it into the state either, as it does seem odd to split it.
|
@jmachuca77 can you take a look at the comments from @WickedShell |
|
Hello, I tried your CAN Drivers with a Smart Tattu 12S battery on a CubePilot Orange but without success. It seems to me that I did not receive any data. Which board did you use for testing it and/or did I miss anything for using it? |
|
@Rataillon sorry about that, I should close this. This branch is actually an older version of the driver. Try this newer version on my branch magicrub/pr/tattu_can3. I think there's still some testing that is pending on it before I make a new PR for it. Please test! |
|
@Rataillon After working with GREPOW directly to support this TattuCAN protocol, we are now helping them implement DroneCAN in their batteries. It is quite close to working. Soon they will only sell DroneCAN batteries and the TattuCAN protocol will be deprecated by GREPOW |
This adds the proprietary CAN driver for TATTU batteries. Tested by @CraigElder and @alexscheck and @jmachuca77.