[JOSS] Reviewer Feedback (sea-bass)

[sea-bass]

Hello,

This contains my review for your entry to the Journal of Open Source Software (JOSS).

SimpleFOC: A Field Oriented Control (FOC) Library for Controlling Brushless Direct Current (BLDC) and Stepper Motors

4232 (https://github.com/openjournals/joss-reviews/issues/4232)

Overall

The Arduino SimpleFOClibrary is a promising project that aims to make motor control education and development accessible at low cost. The solution is well-documented and highly practical from an engineering standpoint. The fact that this provides both software and hardware kits with detailed instructions highlights how complete this solution is.

Based on the above, and the relatively high activity in the GitHub repository itself (stars/forks/issues/contributions), I would recommend this for publication in JOSS with minor revisions.

My only major comment is that the authors are missing unit tests for the C++ software components, and they should be added as appropriate and rolled into the CI pipeline.

Detailed comments

• I would like to highlight that this is not just a software solution (Arduino SimpleFOClibrary), but also a hardware solution (Arduino SimpleFOCShield). Both of them are designed to work together and are documented in the same place, and the shields are available for purchase directly from the authors.
• The software package is well-documented. I especially appreciate that it's not just an API dump, but contains several guided tutorials such as "Getting Started", has example projects, and even a work roadmap to share the vision with the open-source community to gain alignment with potential contributors.
• Activity is great. At the time of review there are 251 forks, 953 stars, 72 PRs (3 open, 69 closed), 97 issues (38 open, 59 closed), and 18 separate contributors (seems like 2 main ones and 16 from the community).
• Installation instructions are complete and span a good spread of expertise -- from "clone this repo, you know how to do that" to "here's how you do in the Terminal" to "forget Git, download a ZIP file".
• Beyond the GitHub repo, there is a SimpleFOC Community forum for user interaction / discussions / support.
• The software Getting Started materials are very complete, and I particularly like the "Simple tests" checkpoints telling users what they should expect to see on the hardware.

Suggested Revisions (Repository)

• My main suggestion is that the repo has automated tests for library compile, but no unit tests for the functionality itself. It would be great to see some unit tests rolled into the CI pipeline.
• I am a little confused in that the repository and documentation points out Arduino, but then the supported hardware lists other platforms and the entire paper/submission also seems platform-agnostic. In fact, the paper makes a point that SimpleFOC supports various microcontroller architectures. So I would suggest making the repo title and documentation reflect this multi-architecture support as much as possible (without renaming the repo, which is always unnecessarily risky).
• A lot of the hardware instructions are relatively fast-moving GIFs that can't be paused/slowed down. Consider also having static pictures with numbered steps, or possibly creating a video (e.g., on YouTube) that users can pause if needed. I see a video tutorial is already on the roadmap.
• The critical sections such as "📢 Make sure to read this before running any motor!" section probably should be called out more clearly as this could cause damage to hardware. Maybe worth elevating to its own section in addition to where they are now?
• It might be good in the documentation to have a graphical description of the software libraries and how they interact with each other. I really like Figures 1 and 2 in the paper and think they should make it into the official doc!
• I would suggest adding a page with contributor guidelines, e.g., "How do I contribute to this project?"
• In src/common/foc_utils.cpp there are several commented-out lines in e.g. the sine and square root approximations that are optimized for other data types. Instead of leaving them commented, is it worthwhile to actually define multiple functions and have users call the right one?

Suggested Revisions (Paper)

• Under "Research Interest", it might be good to also highlight that you are showcasing example projects in your documentation.
• I think the paper should highlight that the authors provide a joint software/hardware solution, and the Arduino shields are available for purchase from the same authors.
• Following the suggested addition of unit tests, it would be nice to briefly discuss how this functionality is validated -- namely, that the authors have a CI pipeline set up that frequently checks that the library compiles and basic functionality performs as intended.

[askuric]

Thanks @sea-bass for your comments, We've integrated most of your proposals to the library and the docs. There are still some that are not completely addressed but are in the pipeline. :smile: