search

grblHAL / iMXRT1062

grblHAL driver for NXP iMXRT1062 (Teensy 4.x)
Other
51 stars 39 forks source link

Sub-par commenting Grbl-Hal sections of code #11

Open AnthonyJQQ opened 3 years ago

AnthonyJQQ commented 3 years ago

I'm somewhat disappointed with the lack of comments the code. Specifically related to the GrblHAL drivers. One of the reasons that I love Grbl, is that the code is very well commented which makes it easy to configure and implement in existing CNC machines.

GrblHAL code should have comments similar to GRBL, but the GRBLHAL components of the code are lacking explanation. For example, I spent some time last night trying to understand what the encoder switch does in the encoder plugin, but the code is so sparsely commented that I was unable to quickly and easily understand how the encoder plugin and functions within the plugin work.

I decided to post this issue to flag everyone working on GrblHAL to remember to properly comment your code so that we can better understand how to use and apply GrblHAL to our real-world machines. The code should have comments before each function explaining what the function does, as well as comments added to the top of pages explaining what the overall code is supposed to do.

Why I'm disappointed: I decided to move a CNC to GrblHAL using Phil Barrett's GrblHal Teensy breakout board The machine originally was operating based on https://github.com/bdurbrow/grbl-Mega. The code in the grbl-Mega project has similar features to the driver features in GrblHal, but it is much better commented and therefore much easier to implement in the real-world. It might be better in my case to use older hardware with well commented code, than to use the latest software/hardware (GrblHAL) due to the lack of code comments. I hope that GrblHAL code evolves to the point of being as awesome and easy to use as Grbl itself!

@terjeio Go easy on me. ;)

Thanks!

terjeio commented 3 years ago

I am sorry to have wasted your precious time.

There is still a lot of work to do - and I simply do not have the time to make everything perfect from the get-go. Sometimes I wish I had kept the code for myself as it takes an incredible amount of time to keep the project going.

I have recently started to add documentation using doxygen to make it more accessible, this will take some time to complete though. The Wiki should be expanded on as well...

Finally, I am happy to hear that the grbl-Mega project is well suited for your needs - it is great to have alternatives available.

andrewmarles commented 3 years ago

I really want to keep positive here, but I decided to comment because I think this 'issue' is incredibly unproductive. Everybody should remember that GRBLHAL is free and open-source, and that it is what you make of it. If you see something you'd like done, you should try to make it happen. Maybe you should spend a bit of time to understand the code and then do a pull request to add some comments, or even just add some stuff in the wiki or readme for the plugin, or post a writeup on your blog about your experience so that others can learn... so many ways that you can contribute that are actually meaningful.

Thank you Terje for this project (and your other ones as well), it is all amazing work and an inspiration for the open-source community.

AnthonyJQQ commented 3 years ago

@terjeio thank you for the link. I'll add some pull requests with comments as I continue to understand the code.

Please don't take this issue the wrong way, I think what you are doing here is amazing, and I am excited to contribute to the project. The link to doxygen is great.

@andrewmarles Code that is poorly commented is an issue in my book. I agree with you that pull requests are a good way to contribute, but they don't alert to a problem.

I wanted to bring this up specifically as an issue. It is something that absolutely needs improvement within the project, and all the devs and contributors should be thinking about it as they are working on their pull requests.