-
-
Notifications
You must be signed in to change notification settings - Fork 41
[REVIEW]: SimpleFOC: A Field Oriented Control (FOC) Library for Controlling Brushless Direct Current (BLDC) and Stepper Motors #4232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks. Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post. For a list of things I can do to help you, just type:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
Checking the BibTeX entries failed with the following error:
|
|
Failed to discover a |
Review checklist for @sea-bassConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Hi @gkthiruvathukal -- I have just gone through the software for my review: simplefoc/Arduino-FOC#174 While the software, documentation, etc. looks great, we are still unable to generate the software paper and therefore review that piece. Could you and the author take a look at this and update us when the paper is available? Additionally, the CI pipeline includes compilation tests, but there are no unit tests, so I have recommended the addition of those for some of the core C++ source. |
@editorialbot generate pdf |
Thanks, @askuric -- turns out the main issue description didn't specify a branch, so it defaulted. I've just edited it to the EDIT: Yep, that worked. I've updated my review and the comment about unit tests is the only major one amidst minor comments. |
Review checklist for @ixjlyonsConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
@gkthiruvathukal (et al.) apologies for the delayed start - I should be able to finish reviewing in the next couple days. |
My review: SimpleFOC is a library offering a modular approach to implementing field oriented control with various hardware components. I'm currently not able to directly verify functionality of the library, but I've been able to successfully compile several examples and have reviewed portions of the code. The code itself, along with the very well-done documentation, appear to be a great resource for implementing advanced motor control for a variety of applications. I recommend accepting. General CommentsI may have missed it, but I haven't come across clear community guidelines as required by JOSS in the documentation or README. I think this is the only blocking issue. Aside from that, I only have a few minor comments:
PaperNo major concerns with the paper. Below are some minor comments:
|
Hey @ixjlyons and @sea-bass, @sea-bass @ixjlyons Regarding the community guidelines. Thanks again for your time and let me know if there is something else that you'd like us to consider! |
Looks good to me 👍
I think that's fine -- my understanding is there just needs to be guidelines written somewhere to make expectations clear. I'd say the links in the header of the docs cover getting help and maybe even bringing up issues. "Please open an issue to propose improvements" in the README would be sufficient I think. Pull request / issue templates are great additions too. |
Of course! I was only suggesting unit testing, as having full-scale (or system-level) testing for a hardware-focused library would be unreasonable. |
Hey @ixjlyons, Also the README was extended with the section about contributing and different community platforms that are available. Finally, I've added the first version of issue templates that should enable users to post more direct and precise questions/issues. For the PRs it is a bit more tricky so we will provide the templates a bit later. |
Great, looks good to me. Unless there's still a hangup on testing, I think that was the last outstanding issue. So this should be ready to go, nice work. |
According to the guidelines, the requirements are: Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified? If there is no plan to create automated unit tests / a CI pipeline for this review process (which is a totally fair decision), could you point me to the "manual steps" that would meet the criteria above? And if not available, could some documentation on testing this software and verifying it works as intended be made? Thanks! |
Hey everyone, I am sorry for a late reply! At the moment the CI pipeline for unit testing of the software components is really not our priority. Mostly because the most of the problems due to the library code comes from the MCU specific issues which cannot really be unit tested. However, we do have guidelines in the docs that explain how each user can verify that each part of the library works correctly with their hardware. This is a very important step for setting up any of the users hardware setups, regardless of how standard or nonstandard they are. So in an way each user really needs to test and verify that the library code works well with their hardware. Additionally we support many different debugging and monitoring features which facilitate testing as well. As basically all of our users use our code only in combination with hardware components, till this point we did not consider putting in place any purely software testing procedures, all of our testing procedures are coupled with hardware components. |
@askuric thank you for clarifying that and pointing me to the right resources -- I think that absolutely meets the testing requirements, and agree with your prioritization especially given the hardware-focused nature of the project. LGTM :) |
@editorialbot set v2.2.2 as version |
Done! version is now v2.2.2 |
@editorialbot generate pdf |
@editorialbot recommend-accept |
|
|
@askuric Please check the MISSING DOIs. Thanks! |
👋 @openjournals/joss-eics, this paper is ready to be accepted and published. Check final proof 👉 openjournals/joss-papers#3297 If the paper PDF and the deposit XML files look good in openjournals/joss-papers#3297, then you can now move forward with accepting the submission by compiling again with the command |
@gkthiruvathukal thanks! |
|
Thanks for the updates. |
@editorialbot generate pdf |
@askuric thanks. Can you please group the |
Ok, done. |
@editorialbot generate pdf |
@editorialbot accept |
|
🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦 |
🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨 Here's what you must now do:
Any issues? Notify your editorial technical team... |
@askuric congratulations on your JOSS paper! Thanks @gkthiruvathukal for editing this work, and thank you @sea-bass and @ixjlyons for your review efforts!! |
🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉 If you would like to include a link to your paper from your README use the following code snippets:
This is how it will look in your documentation: We need your help! The Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:
|
Awesome, thanks @Kevin-Mattheus-Moerman! Thanks @gkthiruvathukal , @sea-bass and @ixjlyons for your reviews and helpful comments, we appreciate it. |
Submitting author: @askuric (Antun Skuric)
Repository: https://github.com/simplefoc/Arduino-FOC/
Branch with paper.md (empty if default branch): joss_paper
Version: v2.2.2
Editor: @gkthiruvathukal
Reviewers: @sea-bass, @ixjlyons
Archive: 10.5281/zenodo.6510536
Status
Status badge code:
Reviewers and authors:
Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)
Reviewer instructions & questions
@sea-bass & @ixjlyons, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review.
First of all you need to run this command in a separate comment to create the checklist:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @gkthiruvathukal know.
✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨
Checklists
📝 Checklist for @sea-bass
📝 Checklist for @ixjlyons
The text was updated successfully, but these errors were encountered: