Dynamixel sdk playing with sync read write #141
Conversation
…bjects Currently if you global Sync Read or write as a global object, it will crash the processor, as you can not rely on when the constructor is called. Also the passed in port and protocol pointers are not set yet. So created a version of the constuctor that you don't pass these two pointers in, but instead call init(port, ph), to fill it in during setup. My test case works with this :D
Experiment - to update some portions of the Dynamixel SDK to minimize or remove all need memory allocations, either through malloc or new. First one GroupSyncWrite - You can specify max number of items in constructor. There is also second form of constructor, for two part initialize. Needed if you wish to make global object, as you don't have port/protocol yet, plus issues with when C++ constructors are called. Also added some new features to allow you to set items, which are part of the write, in same way as you can read items on Sync_Read. Added an example program that makes use of these features. Where I can do Sync Write to N servos (currently defined as 2), and set velocity and position, and then the Sync Read can get the position, velocity and load. Currently setup for XL430... servos
Like the prevous update, changed group_sync_read to work with just one allocated buffer. Calling program also has the option to set this buffer as to be able to statically define memory usages. Some updates to the Sync_write code in previous commit to move all of the memory allocation and adding items to the look function. Also have a value defined of how much to add memory the user needs to add for each servo item to the count of bytes returned or set in the Sync read or write. Again test case updated
…namixelWorkbench/t_Find_Dynamixel/t_Find_Dynamixel.ino Workbench: Find Dynamixels test make more Arduino like. Move code to loop, which allows you to rerun the test, which I like as sometimes I forget to turn on the Servo power and the servos are not found. Also changed the printing of servo info to be more Arduino. That is simply use simple Serial.print functions instead of generating and combing string.
Instead of hard coding 16, give it a name as part of an enum statement, so the user can simply change the enum value and everything is updated. Don't like magic numbers
|
Hi @KurtE :) Thanks, |
|
Wondering about the differences in the sets of files. (OpenCR/OpenCM) versus Dynamixel_SDK project. There are some simple differences. Things like copyright messages and the like. The main functionality difference I see is with the Protocol(1/2)PacketHandler::readRX, the main SDK version you pass in an ID to verify that the packet was from an ID and the Open... versions do not and simply grab the next message. |
|
Originally, the two codes should be the same. |
|
@KurtE , SDK has been updated with 1.4.0 release. Would you like to check it out? |
|
Hi @OpusK and @kijongGil , As I mentioned in other one. Sort of tied up. We were without power for about 24 hours... Luckily it is back... I tried compiling one of the Examples I did for testing the Sync read and write and it currently does not compile. That is lines like: My guess is these function names were updated, probably to: Note: if so the file keywords.txt was not updated to the new method names... |
|
Hi again and happy Holidays @OpusK and @kijongGil , Actually looks Dynamixel Workbench API for SyncRead and SyncWrite is more significantly changed. Need to figure out the new proper semantics. That is before I had a quick and dirty test program, that added the one SyncWriteHandler and 3 SyncReadHandler. It only called the addSyncWriteHandler and it was assumed that it applied to all pinged servos... But the new one you pass in an address. Do you only call this once? And the ID is just so you know can map it to control table to then lookup the String and convert to address? Or do you call this for each ID you wish to do the SyncRead (or write) on? The new syncRead and syncWrite methods, have an index you pass in? Where do you get this index? But oops I see that this PR/Issue is about the SDK not the workbench... So maybe should remove these comments? NOW back to this PR request... As for this PR, I think it is more or less toast... That is it looks like the current stuff still uses all of the heavy weight stuff: Example look at the start of GroupSyncRead The code is still doing lots of new/delete and the like. Likewise: So again using vector and map which will again do lots of New/delete calls. It also has duplicated info? Again if interested, the PR could be redone again to the current code base. Not sure what all changed. But I know that none of the memory reductions appears to be in the current stuff that was released. Kurt |
|
Hi, @KurtE. Happy Holidays!!
Is it compiled in OpenCR 1.4.1? In which example did you get this error? |
|
Hi @OpusK and again Happy Holidays, Sorry I may have used poor choice of words, by saying Example. What I tested was my own test program based on the earlier release of OpenCM/OpenCR boards. The issue is more of, what are your policy on compatibility between releases? That is a customer develops an application, using Dynamixel SDK or Dynamixel Workbench and then there is a new release of the Arduino boards for OpenCM or OpenCR, should their existing programs continue to compile and run. And if not, should the customer then have to add #ifdef code in depending on release number (assuming there is a defined value to check), so that their code will build for both the old and the new as maybe some of their customers may not have updated yet to new release... I tried asking that over on Dynamixel Workbench project in an issue which is now closed: ROBOTIS-GIT/dynamixel-workbench#206 Again this probably should be split off to own issue/PR, luckily I don't think it impacts most of my own stuff as I use the SDK and not workbench. As for this PR, again looks like none of the memory usage things have been included in this release. Also wonder now if these changes are wanted if I would need to create a new PR, and try to fold the changes from this branch into the current code base... |
|
Hi, @KurtE, Sorry for late reply and any inconvenience.
There seems to be a clear policy on this and some things that are not. |
|
Not a problem - Currently I am busy doing some stuff for new Beta Teensy board 4.0... Will get back to robotics stuff soon |
|
Our team is also interested in seeing improved sync read/write performance merged. Semantic Versioning the SDK would probably help. |
Put up this Pull Request, that tries to reduce or eliminate any memory allocations done as part of group_sync_read or group_sync_write.
They both create a single buffer, that holds all of the data. It also allows the using program to statically define this buffer and tell the code to use their buffer instead. This allows programs to get a better understanding of exactly how much memory they are using.
Also it removes all of the calls to heap manager. (Except to allocate and free the one buffer)
I also added a method to sync write that worked like Sync Read, such that you could pass in an uint32 value and size of your field and it would do the appropriate packing.
So far I have only done these two objects. Something similar could be done for the Bulk read and write.
If this level of change looks valid to you, I can also migrate this code over to both the SDK project as well as OpenCM.
This pull also includes an extended Sync Read and write example that the Sync Write updates both position and velocity and reads in position, velocity and force assuming XL430... servos.
I may have also semi accidentally checked in my Dynamixel workbench test (find servos) update that made it more Arduino ish, like using Serial.print(...) function instead of concatenating strings. Also moved code to loop, to allow it to run again. And maybe a debug message I had which printed out the size of the workbench object.
Let me know if you like these changes or if you think parts of it should change.