Wiki - 3.1 Source File: Folder tree diagram has protocol 2.0 misspelled as protocol 1.0 under example folder

Shreeyak commented 7 years ago

Under the example folder, protocol 2.0 misspelled:

LeonJung commented 7 years ago

@Shreeyak

Hi, and thank you for contribution :)

What you pointed out is modified just now

Shreeyak commented 7 years ago

Hey, You're welcome! Glad to help. I also wanted to contribute some other minor changes. Don't know if you'll get this mail if I reply to it. Ping me back if you do?

I want there to be more comments in the source code so that people can understand it.

On Thu, Apr 13, 2017 at 6:41 AM, Leon notifications@github.com wrote:

@Shreeyak https://github.com/Shreeyak

Hi, and thank you for contribution :)

What you pointed out is modified just now

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ROBOTIS-GIT/DynamixelSDK/issues/70#issuecomment-293753425, or mute the thread https://github.com/notifications/unsubscribe-auth/AMNQuYIRa_dWMUY-RqHrx8T1H5aavn2Hks5rvX1RgaJpZM4M22vY .

LeonJung commented 7 years ago

@Shreeyak

Of course, keep going

Shreeyak commented 7 years ago

Yeah, so I'm working on porting this library to a custom SoC running a version of linux.

So, was going through the source code and found a severe lack of comments, which made understanding the code a bit hard, given the extremely similar function names. So, I've been commenting the code for my own documentation and would soon like to submit those as a pull request.
Comments throughout the code will make it much simpler for anyone else going through the code to understand it. I would also like to add to the documentation a section for developers that explains each of the source files and their hierarchy.
I also find the extremely similar function names (which call each other) extremely confusing. Is there any chance of looking at re-naming said functions in the official git repo? I'll send examples soon.

On Fri, Apr 14, 2017 at 11:18 AM, Leon notifications@github.com wrote:

Of course, keep going

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ROBOTIS-GIT/DynamixelSDK/issues/70#issuecomment-294098418, or mute the thread https://github.com/notifications/unsubscribe-auth/AMNQuSbViF7mRoboNAXHZiBv7k0FpZddks5rvw_HgaJpZM4M22vY .

LeonJung commented 7 years ago

@Shreeyak

You mean like intel edison board - yacto linux, for example?

   - So, was going through the source code and found a severe lack of
   comments, which made understanding the code a bit hard, given the extremely
   similar function names. So, I've been commenting the code for my own
   documentation and would soon like to submit those as a pull request.

   - Comments throughout the code will make it much simpler for anyone else
   going through the code to understand it. I would also like to add to the
   documentation a section for developers that explains each of the source
   files and their hierarchy.

Yeah, that's a good point. Since I should manage whole languages of DynamixelSDK, the comments should be written in a general ways. I'm glad to see how the source has been reflected by users, but be sure that you are working on the develop branch. pull requests to the master were rejected because it was hard to manage currently.

   - I also find the extremely similar function names (which call each
   other) extremely confusing. Is there any chance of looking at re-naming
   said functions in the official git repo? I'll send examples soon.

Of course, let me take a look your suggestions. I'll take it all or partly when some are against the rules of DynamixelSDK management, but anyway.

Shreeyak commented 7 years ago

Hi,

Yes, it's similar to the edison, in that it's an SoC, but it's not one of the models available in the consumer market, like edison or yolo. It's a proprietary design, running a stripped down android.
I'm still working on first getting this integrated into our build, so will start working on improving it once that is done. Before I make any changes, however, can you share the DynamixelSDK management rules? It will be much easier to work on code keeping the rules in mind.
Some examples, for confusing naming, are as below:

//protocol1_packet_handler1.h WINDECLSPEC void txPacket1 (int port_num); WINDECLSPEC void rxPacket1 (int port_num); WINDECLSPEC void txRxPacket1 (int port_num); . . . WINDECLSPEC void readTx1 (int port_num, uint8_t id, uint16_t address, uint16_t length); WINDECLSPEC void readRx1 (int port_num, uint16_t length); WINDECLSPEC void readTxRx1 (int port_num, uint8_t id, uint16_t address, uint16_t length);

The correlation between these functions is not clear at all and takes a lot of digging to figure out. Also, seems conflicting, eg: what is readTx1? If its only Tx, how can one read????

On Sun, Apr 16, 2017 at 11:08 AM, Leon notifications@github.com wrote:

@Shreeyak https://github.com/Shreeyak

You mean like intel edison board - yacto linux, for example?

So, was going through the source code and found a severe lack of comments, which made understanding the code a bit hard, given the extremely similar function names. So, I've been commenting the code for my own documentation and would soon like to submit those as a pull request. - Comments throughout the code will make it much simpler for anyone else going through the code to understand it. I would also like to add to the documentation a section for developers that explains each of the source files and their hierarchy.

Yeah, that's a good point. Since I should manage whole languages of DynamixelSDK, the comments must be written in a general ways. I'm glad to see how the source has been reflected by users, but be sure that you are working on the develop branch. pull requests to the master were rejected because it was hard to manage currently.

I also find the extremely similar function names (which call each other) extremely confusing. Is there any chance of looking at re-naming said functions in the official git repo? I'll send examples soon.

Of course, let me take a look your suggestions. I'll take it all or partly when some are against the rules of DynamixelSDK management, but anyway.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ROBOTIS-GIT/DynamixelSDK/issues/70#issuecomment-294334858, or mute the thread https://github.com/notifications/unsubscribe-auth/AMNQuVppniDj3rVHBXlEIqWMt-an4C_Pks5rwalvgaJpZM4M22vY .

LeonJung commented 7 years ago

Hi,

 - Yes, it's similar to the edison, in that it's an SoC, but it's not one of the models available in the consumer market, like edison or yolo. It's a proprietary design, running a stripped down android.

Well, I had tested the SDK in the Raspberry Pi under Snappy Ubuntu Core, so your board must have work with it.

 - I'm still working on first getting this integrated into our build, so will start working on improving it once that is done. Before I make any changes, however, can you share the DynamixelSDK management rules? It will be much easier to work on code keeping the rules in mind.

If you want to just comment on the source about what every functions mean, theres no specific rules so show me an example source codes which has your comments, and let's work in the rest of the codes as same as it showed after my review.

If you want to change the APIs, you should show me every point to point, and will take time.

 - Some examples, for confusing naming, are as below: 
The correlation between these functions is not clear at all and takes a lot of digging to figure out. Also, seems conflicting, eg: what is readTx1? If its only Tx, how can one read????

readTx does only Transmit the package with 'Read' instruction. It provides readTxRx but also provides readTx and readRx separately because they can be used in thread process, but normally the users will readTxRx which waits the returned package during designated time.

and the number 1 means the protocol: Dynamixel, as you know, works under communication protocol 1 or 2 or both (but not the same time) depended on the models.

If you mean that txPacket and readTx make confusion, actually readTxPacket looks much better, but we thought that theres no needs for doing it.

But, we have been monitoring the needs and will follow if there are.

Plus, again, upload your sources into the develop branch when your works are done.

Shreeyak commented 7 years ago

Hi,

I have managed to get the source code compiled with my Hardware. Having issues in my hardware in switching from read mode to write mode (we are using a tri state buffer for half duplexing). I will get started on the points you mentioned above once I have fully integrated the SDK into my build.
I also now need to port the Dynamixel SDK onto a MCU. I am currently using an STM32 Nucleo board for testing. I have built an interrupts-based half-duplex UART library with rx/tx ring buffers for the STM. Now I think all I need to to create a new platform check (similar to _linux64) and write functions for setupport, readport, writeport. Can you suggest the best way to get started on porting the SDK to the MCU? I would love to generate a generalized solution that can be merged into the SDK and can be used by others.
Please do send the SDK management rules so I can write code that follows the rules.

Thanks!

On Apr 21, 2017 7:21 AM, "Leon" notifications@github.com wrote:

Hi,

Yes, it's similar to the edison, in that it's an SoC, but it's not one of the models available in the consumer market, like edison or yolo. It's a proprietary design, running a stripped down android.

Well, I had tested the SDK in the Raspberry Pi under Snappy Ubuntu Core, so your board must have work with it.

I'm still working on first getting this integrated into our build, so will start working on improving it once that is done. Before I make any changes, however, can you share the DynamixelSDK management rules? It will be much easier to work on code keeping the rules in mind.

If you want to just comment on the source about what every functions mean, theres no specific rules so show me an example source codes which has your comments, and let's work in the rest of the codes as same as it showed after my review.

If you want to change the APIs, you should show me every point to point, and will take time.

Some examples, for confusing naming, are as below: The correlation between these functions is not clear at all and takes a lot of digging to figure out. Also, seems conflicting, eg: what is readTx1? If its only Tx, how can one read????

readTx does only Transmit the package with 'Read' instruction. It provides readTxRx but also provides readTx and readRx separately because they can be used in thread process, but normally the users will readTxRx which waits the returned package during designated time.

and the number 1 means the protocol: Dynamixel, as you know, works under communication protocol 1 or 2 or both (but not the same time) depended on the models.

If you mean that txPacket and readTx make confusion, actually readTxPacket looks much better, but we thought that theres no needs for doing it.

But, we have been monitoring the needs and will follow if there are.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ROBOTIS-GIT/DynamixelSDK/issues/70#issuecomment-296010678, or mute the thread https://github.com/notifications/unsubscribe-auth/AMNQucmdzOdS9-vimwvMiwvtcBZJCLkPks5ryAu5gaJpZM4M22vY .

LeonJung commented 7 years ago

1. I have managed to get the source code compiled with my Hardware. Having
issues in my hardware in switching from read mode to write mode (we are
using a tri state buffer for half duplexing). I will get started on the
points you mentioned above once I have fully integrated the SDK into my
build.

Good Luck.

2. I also now need to port the Dynamixel SDK onto a MCU. I am currently
using an STM32 Nucleo board for testing.
I have built an interrupts-based half-duplex UART library with rx/tx ring
buffers for the STM. Now I think all I need to to create a new platform
check (similar to _linux64) and write functions for setupport, readport,
writeport.
Can you suggest the best way to get started on porting the SDK to the MCU?
I would love to generate a generalized solution that can be merged into the
SDK and can be used by others.

I suggest you to see how the DynamixelSDK can be used in Arduino Platform. System is slightly different then as you think, but I guess it will help you.

3. Please do send the SDK management rules so I can write code that follows
the rules.

My team will talk about the open source manual sooner, so then I will notice you

Shreeyak commented 7 years ago

I just went back to the github wiki and noticed that it is written "Arduino (coming soon)".

Do you suggest I wait until your team implements it on an Arduino? Any way I can pitch in?

On Sun, Apr 23, 2017 at 5:01 PM, Leon notifications@github.com wrote:

I have managed to get the source code compiled with my Hardware. Having issues in my hardware in switching from read mode to write mode (we are using a tri state buffer for half duplexing). I will get started on the points you mentioned above once I have fully integrated the SDK into my build.

Good Luck.

I also now need to port the Dynamixel SDK onto a MCU. I am currently using an STM32 Nucleo board for testing. I have built an interrupts-based half-duplex UART library with rx/tx ring buffers for the STM. Now I think all I need to to create a new platform check (similar to _linux64) and write functions for setupport, readport, writeport. Can you suggest the best way to get started on porting the SDK to the MCU? I would love to generate a generalized solution that can be merged into the SDK and can be used by others.

I suggest you to see how the DynamixelSDK can be used in Arduino Platform. System is slightly different then as you think, but I guess it will help you.

Please do send the SDK management rules so I can write code that follows the rules.

My team will talk about the open source manual sooner, so then I will notice you

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ROBOTIS-GIT/DynamixelSDK/issues/70#issuecomment-296438471, or mute the thread https://github.com/notifications/unsubscribe-auth/AMNQuV8HT3d-zLY22ARdGgll2Ao-Ltfsks5ryz2RgaJpZM4M22vY .

LeonJung commented 7 years ago

That means, we are now developing Arduino version SDK for all Arduino compatible boards.

What I linked is an Arduino version Dynamixel SDK only for OpenCR (it is our product but is compatible with Arduino), and I told you because it is not as same as SDK for MCU you mentioned but is as similar as that, and I thought that it will be a help for your work.

Shreeyak commented 7 years ago

Hi! I think there might've been an error. There is no link.

Is it in the opencr git repo?

On Apr 24, 2017 5:59 AM, "Leon" notifications@github.com wrote:

That means, we are now developing Arduino version SDK for all Arduino compatible boards.

What I linked is an Arduino version Dynamixel SDK only for OpenCR (it is our product but is compatible with Arduino), and I told you because it is not as same as SDK for MCU you mentioned but is as similar as that, and I thought that it will be a help for your work.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ROBOTIS-GIT/DynamixelSDK/issues/70#issuecomment-296500210, or mute the thread https://github.com/notifications/unsubscribe-auth/AMNQuQV8jtOnM2_BrlcMVG_z8RAwUuSnks5ry-zWgaJpZM4M22vY .

LeonJung commented 7 years ago

@Shreeyak

yes, it is in there

Shreeyak commented 7 years ago

Thanks, I'll check it out!!

On Apr 24, 2017 10:09 AM, "Leon" notifications@github.com wrote:

@Shreeyak https://github.com/Shreeyak

yes, it is in there

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ROBOTIS-GIT/DynamixelSDK/issues/70#issuecomment-296523137, or mute the thread https://github.com/notifications/unsubscribe-auth/AMNQubjgWhLUuV1k-qZwZn2plFtZ29z5ks5rzCdlgaJpZM4M22vY .

LeonJung commented 7 years ago

@Shreeyak

GL

LeonJung commented 7 years ago

@Shreeyak

Hi,

I'll soon provide DynamixelSDK for Arduino here. Reopen this issue if you have interest.

Soon I will pull the source from develop-mid branch to the master

Shreeyak commented 7 years ago

Definitely interested! Glad to see such fast developments at Robotis. Great work!

On Tue, Jul 11, 2017 at 6:30 PM, Leon notifications@github.com wrote:

Closed #70 https://github.com/ROBOTIS-GIT/DynamixelSDK/issues/70.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ROBOTIS-GIT/DynamixelSDK/issues/70#event-1158733326, or mute the thread https://github.com/notifications/unsubscribe-auth/AMNQuRCuigqR9Zeef9_th4ca5Q18Rlmbks5sM3HygaJpZM4M22vY .

ROBOTIS-GIT / DynamixelSDK

Wiki - 3.1 Source File: Folder tree diagram has protocol 2.0 misspelled as protocol 1.0 under example folder #70