Submitting author: @adi3 (Adi Singh)
Repository: https://gitlab.com/toposens/public/ros-packages
Version: v1.2.1
Editor: @gkthiruvathukal
Reviewer: @dvberkel
Archive: 10.5281/zenodo.3336380
Status badge code:
HTML: <a href="http://joss.theoj.org/papers/1e8261fd1d20b2521ef8735d2da50b6b"><img src="http://joss.theoj.org/papers/1e8261fd1d20b2521ef8735d2da50b6b/status.svg"></a>
Markdown: [](http://joss.theoj.org/papers/1e8261fd1d20b2521ef8735d2da50b6b)
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.)
@dvberkel, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @gkthiruvathukal know.
โจ Please try and complete your review in the next two weeks โจ
paper.md
file include a list of authors with their affiliations?Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @dvberkel it looks like you're currently assigned to review this paper :tada:.
:star: Important :star:
If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews ๐ฟ
To fix this do the following two things:
For a list of things I can do to help you, just type:
@whedon commands
Attempting PDF compilation. Reticulating splines etc...
PDF failed to compile for issue #1531 with the following error:
Can't find any papers to compile :-(
@whedon generate pdf from branch joss_paper
Attempting PDF compilation from custom branch joss_paper. Reticulating splines etc...
@gkthiruvathukal @dvberkel Thanks for getting the review started :)
Shouldn't it mention @nnadeau as a reviewer too? Or are the reviews done sequentially?
@ts-adi I am looking for community guidelines in your repository. But I can't find them. Am I missing something, or is a CONTRIBUTING.md
file missing?
@gkthiruvathukal I don't have a Toposens TS3 sensor, so how can I can I review if the functionality claims of the software?
@ts-adi Is there a possibility to send a sensor for me to review?
@dvberkel Just merged master changes to the joss_paper branch. We've taken away most of the documentation from the readme and put it on the ROS wiki (and provided a link to it from the git readme). See here:
This avoids repetition of the same content in two places. Our wiki page mentions (and links to) the repo's bug/feature tracker. I haven't explicitly added a CONTRIBUTING.md
file because instructions to contribute our laid by the ROS wiki already here: http://wiki.ros.org/Contributing
Do you suppose adding a link to this in our readme should suffice? I would much rather add content to the readme rather than add a separate file.
@gkthiruvathukal What is the procedure for testing open source software built for commercial hardware devices?
We do have an extensive automated test suite in the repo that emulates the sensor (straightforward posix emulation) for our continuous integration needs. These tests run on a docker container and as such have no direct access to the hardware. Through emulation, we ensure tests can run hardware-free and still confirm that our functionality is not broken. Do you suppose going through the test suite might assist in confirming our functionality claims?
@dvberkel Let's see what suggestions come from the editor. We'll find a way forward accordingly :)
@dvberkel Just merged master changes to the joss_paper branch. We've taken away most of the documentation from the readme and put it on the ROS wiki (and provided a link to it from the git readme). See here:
* https://gitlab.com/toposens/public/ros-packages/tree/joss_paper * http://wiki.ros.org/toposens
This avoids repetition of the same content in two places. Our wiki page mentions (and links to) the repo's bug/feature tracker. I haven't explicitly added a
CONTRIBUTING.md
file because instructions to contribute our laid by the ROS wiki already here: http://wiki.ros.org/Contributing
I am all in favor of not duplicating information :+1:
Do you suppose adding a link to this in our readme should suffice? I would much rather add content to the readme rather than add a separate file.
For me a link would more than suffice.
@ts-adi and @dvberkel, To answer this question fully, I would need to know whether the commercial devices are available to the public or are still in the lab. If there is an emulation layer (that is, either a library or a hardware emulator) that can be used to run the tests, I think this would be satisfactory.
Ultimately, what I would be checking is whether there is a strong emphasis on testing, which in many cases can also be done by inspection. So if I am understanding it right: (a) we have proper unit tests / system tests, and (b) we can run the tests using an emulation layer within a Docker container? If both of these apply--as I think they do--this more than meets our editorial standards.
Lastly, is there some sort of continuous integration process that you use, @ts-adi, where you might be able to show the results of building and testing the software on your own hardware? This would give us an additional level of confidence.
Thanks for checking with me! Software engineering is my thing, and I always enjoy hearing about interesting cases like this one, which I think will be of interest to JOSS readers in general.
@gkthiruvathukal there is an extensive continuous integration process that ticks both your requirements.
I will dive into the looking into the tests more thoroughly
@gkthiruvathukal thank you for the detailed guidance. Here are specific answers to your questions:
The device is available to the public: http://toposens.com/ts3
We have tests with a wide code coverage and they run within a docker container using the emulation. Apart from our own CI setup in gitlab, these tests are automatically triggered when an update is merged in a ROS distro. See results of our 42 tests here. If you click on the green label saying "Continuous Integration", you can also view individual Jenkins jobs on the ROS buildfarm.
As for as continuous testing of the software on real hardware is concerned, this is how we do it:
Anytime there is a firmware update on the device, we record data dumps coming from the sensor and upload it to a remote server: http://85.214.248.206/ros/
Here is an example of this recording where we inform this Makefile that a device is plugged in:
````cmake
if(PLUGGED_IN)
set(LOG_PERIOD 3s)
set(PORT /dev/ttyUSB0)
message(STATUS "Extracting data from live stream for ${LOG_PERIOD}")
execute_process(
COMMAND timeout ${LOG_PERIOD} roslaunch ${PROJECT_NAME} ${PROJECT_NAME}.launch
COMMAND timeout ${LOG_PERIOD} cat ${PORT}
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
OUTPUT_FILE raw_stream
)
message(STATUS "Raw stream extraction complete")
endif()
````
catkin_download_test_data(
${PROJECT_NAME}_raw_stream
${TEST_DATA_LOCATION}/raw_stream
DESTINATION ${CATKIN_DEVEL_PREFIX}/${CATKIN_PACKAGE_SHARE_DESTINATION}/tests
MD5 3ec2f56b1cf5372cc98499c4458d37ff
)
catkin_download_test_data(
${PROJECT_NAME}_ts_scans.bag
${TEST_DATA_LOCATION}/ts_scans.bag
DESTINATION ${CATKIN_DEVEL_PREFIX}/${CATKIN_PACKAGE_SHARE_DESTINATION}/tests
MD5 1fa17f252671369fa754e5e0736dbb7c
)
This process keep us confident that hardware and firmware updates ultimately do not break our codebase since rosbag
files hold the entire state of the ROS system (all comms, all frequencies, all device info) for the duration of a recording.
Hope this gives a little more clarity on the project and helps you wade through our tests @dvberkel
@gkthiruvathukal By the way, wanted to follow up on my earlier question about the second reviewer. Should he be assigned to this review as well?
@dvberkel I have also added a few lines at the end of the readme to satisfy the community guidelines condition as per our discussion above: https://gitlab.com/toposens/public/ros-packages/tree/joss_paper
@ts-adi I went with one reviewer on this submission because I understand the subject matter well and can fill in missing details (as I was able to do on the testing matter). I feel like we're moving in the direction of acceptance and are getting great feedback from our reviewer, so are you on with continuing with just one, union input from me?
@dvberkel I have also added a few lines at the end of the readme to satisfy the _community guidelines_ condition as per our discussion above: https://gitlab.com/toposens/public/ros-packages/tree/joss_paper
This is excellent!
@ts-adi I went with one reviewer on this submission because I understand the subject matter well and can fill in missing details (as I was able to do on the testing matter). I feel like we're moving in the direction of acceptance and are getting great feedback from our reviewer, so are you on with continuing with just one, union input from me?
@gkthiruvathukal Yes, this sounds reasonable to me. Thanks!
@ts-adi I am not very familiar with Robotic Operating System. I have read some of the tutorial material. My understanding of what your project entails is that it offers a standard interface to read sensor data.
Am i correct in my assumption that the for the * Functionality documentation* part of the review. I should review that it adheres to the ROS framework guidelines? I.e. there is no documentation other than expect to fit in with other ROS sensors and data?
@dvberkel Your understanding is correct. It standardizes the following things:
No other documentation to go through except what we have in the ROS wiki (http://wiki.ros.org/toposens) and the general ROS framework guidelines. However, what I understand from the editor's previous comments is that you should simply ensure that functionality of the libraries is adequately tested by the automated testing suite.
@dvberkel Your understanding is correct. It standardizes the following things:
* reading and parsing incoming data from a TS3 sensor * fine tuning wave or signal processing parameters directly from ROS * demonstrates integration with popular ROS tools like rviz, pcl2, tf2, etc
No other documentation to go through except what we have in the ROS wiki (http://wiki.ros.org/toposens) and the general ROS framework guidelines.
Great :+1:
However, what I understand from the editor's previous comments is that you should simply ensure that functionality of the libraries is adequately tested by the automated testing suite.
This is true, but there is also a review point on the documentation side.
@dvberkel How was your experience with the software? Can I help you anywhere with the installation or running a functionality? I know ROS has a pretty steep learning curve.
@gkthiruvathukal @dvberkel Can we get an update on this when you have a moment please?
@ts-adi I am satisfied with your responses to my earlier questions about testing. I'm confident that there is a good process behind this work (from a software engineering standpoint).
I'm looking through the comments to see whether @dvberkel's issues are all addressed, so we can move toward acceptance. Upon hearing from @dvberkel, the next step will be to have you create a Zenodo archive for the submission.
Sorry for the late reply. I was moving and was unexpectedly longer than anticipated without internet. I have not reservation about the functionality.
The only thing that I haven't checked is the references. But I think whedon can check those automatically? @gkthiruvathukal Is this correct.
@dvberkel Thanks for getting back on this (and hope your move went smooth). I'll try to run the whedon command and see what it produces.
@whedon check references
Attempting to check references...
Doesn't look like it worked. @gkthiruvathukal any pointers?
Doesn't look like it worked. @gkthiruvathukal any pointers?
I don't think @whedon knows how to check references when the paper isn't on the default (master
) branch sorry.
@arfon How should the reviewers check for the references then? Would it make sense if I temporarily commit the paper (and .bib) to the master branch?
A separate question for @arfon and @gkthiruvathukal This paper review was started when the software was at version 1.1.0. Since then, we have added a few minor things and fixed bugs and the repo is currently at version 1.2.1 (no effect on paper content though). So when I index the software on Zenodo, would it be for v1.1.0 or v1.2.1?
@whedon check references from branch joss_paper
Attempting to check references...
@whedon check references from branch joss_paper
Attempting to check references...
@whedon check references from branch joss_paper
Attempting to check references... from custom branch joss_paper
```Reference check summary:
OK DOIs
MISSING DOIs
INVALID DOIs
@arfon How should the reviewers check for the references then? Would it make sense if I temporarily commit the paper (and .bib) to the master branch?
I've added this to @whedon's capabilities now :point_up:
Since then, we have added a few minor things and fixed bugs and the repo is currently at version 1.2.1 (no effect on paper content though). So when I index the software on Zenodo, would it be for v1.1.0 or v1.2.1?
I think it's fine to archive v1.2.1
```Reference check summary:
OK DOIs
MISSING DOIs
INVALID DOIs
```Reference check summary:
OK DOIs
MISSING DOIs
INVALID DOIs
@arfon Great work extending whedon's functionality so quickly!
@gkthiruvathukal @dvberkel Looks like references all check out
@arfon @gkthiruvathukal Currently in the process of archiving the submission on Zenodo. Should I index the software release from the master
branch? Or should this be from the custom joss_paper
branch that contains the contents of master
+ the JOSS paper?
@ts-adi, If the plan is to continue development and tagging releases from master, I would use master. You don't need to keep the JOSS paper in your archive. Thanks!
@gkthiruvathukal The submission is now archived: https://zenodo.org/record/3336380#.XS3VwvyxVhE
JOSS requires the DOI, not the URL...
Ok, here you go: 10.5281/zenodo.3336380
I am giving the all green from my side :+1:
@whedon commands
Here are some things you can ask me to do:
# List all of Whedon's capabilities
@whedon commands
# Assign a GitHub user as the sole reviewer of this submission
@whedon assign @username as reviewer
# Add a GitHub user to the reviewers of this submission
@whedon add @username as reviewer
# Remove a GitHub user from the reviewers of this submission
@whedon remove @username as reviewer
# List of editor GitHub usernames
@whedon list editors
# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers
# Change editorial assignment
@whedon assign @username as editor
# Set the software archive DOI at the top of the issue e.g.
@whedon set 10.0000/zenodo.00000 as archive
# Set the software version at the top of the issue e.g.
@whedon set v1.0.1 as version
# Open the review issue
@whedon start review
EDITORIAL TASKS
# Compile the paper
@whedon generate pdf
# Compile the paper from alternative branch
@whedon generate pdf from branch custom-branch-name
# Remind an author or reviewer to return to a review after a
# certain period of time (supported units days and weeks)
@whedon remind @reviewer in 2 weeks
# Ask Whedon to accept the paper and deposit with Crossref
@whedon accept
# Ask Whedon to check the references for missing DOIs
@whedon check references
@whedon set 10.5281/zenodo.3336380 as archive
OK. 10.5281/zenodo.3336380 is the archive.
Appreciate all your help with this @dvberkel !
@gkthiruvathukal thanks for setting the archive. What happens next?
@ts-adi I will follow up shortly. We have a few final checks, which I will post when I get back to work tomorrow. We should have this wrapped up soon.
Ok, so here is the final checklist. Can you please do the following? (This is something I am doing for all submissions now, so you can just check the ones you already did above.)
Once these are done, I will do a final proofread and sign off on the submission.
@whedon set v1.2.1 as version
I'm sorry @ts-adi, I'm afraid I can't do that. That's something only editors are allowed to do.
@gkthiruvathukal Here's the checklist. The only remaining step as I see it is setting the version here to 1.2.1 to match the zenodo archive. Looks like only you can instruct @whedon to do so.
[x] Make a tagged release of your software, and list the version tag of the archived version here.
[x] Archive the reviewed software in Zenodo
[x] Check the Zenodo deposit has the correct metadata, this includes the title (should match the paper title) and author list (make sure the list is correct and people who only made a small fix are not on it); you may also add the authors' ORCID.
[x] List the Zenodo DOI of the archived version here.
@whedon set v1.2.1 as version
OK. v1.2.1 is the version.
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
PDF failed to compile for issue #1531 with the following error:
Can't find any papers to compile :-(
@whedon generate pdf from branch joss_paper
Attempting PDF compilation from custom branch joss_paper
. Reticulating splines etc...
PDF failed to compile for issue #1531 with the following error:
error: pathspec 'joss_paper
' did not match any file(s) known to git.
Can't find any papers to compile :-(
@ts-adi Are you able to generate the paper PDF? I cannot seem to do so now. I'm going to see if the @openjournals/joss-eics can help us figure this out.
@whedon generate pdf from branch joss_paper
Attempting PDF compilation from custom branch joss_paper. Reticulating splines etc...
@ts-adi Are you able to generate the paper PDF? I cannot seem to do so now. I'm going to see if the @openjournals/joss-eics can help us figure this out.
@gkthiruvathukal looks like there was an extraneous /n character in the command you issued. I've generated the pdf above. Feel free to give it a shot yourself too.
@whedon generate pdf from branch joss_paper
Attempting PDF compilation from custom branch joss_paper. Reticulating splines etc...
@ts-adi No idea how that extraneous newline got in there. All appears to be in working order now. I've checked the proof and everything appears to be fine from my checklist.
@openjournals/joss-eics I recommend this submission for acceptance.
Hi @adi3 @ts-adi I just submitted a minor PR that made some edits to the bib file, could you merge that? https://gitlab.com/toposens/public/ros-packages/merge_requests/83
@kyleniemeyer done!
@whedon generate pdf from branch joss_paper
@whedon generate pdf from branch joss_paper
Attempting PDF compilation from custom branch joss_paper. Reticulating splines etc...
@ts-adi ah sorry, one more little fix in https://gitlab.com/toposens/public/ros-packages/merge_requests/84
@kyleniemeyer Also merged now. Appreciate your due diligence with this.
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
PDF failed to compile for issue #1531 with the following error:
Can't find any papers to compile :-(
@whedon generate pdf from branch joss_paper
@whedon generate pdf from branch joss_paper
Attempting PDF compilation from custom branch joss_paper. Reticulating splines etc...
Attempting PDF compilation from custom branch joss_paper. Reticulating splines etc...
OK, all looks good, so I'll proceed with accepting.
@whedon accept
Attempting dry run of processing paper acceptance...
Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/854
If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/854, then you can now move forward with accepting the submission by compiling again with the flag deposit=true
e.g.
@whedon accept deposit=true
@whedon accept deposit=true
Doing it live! Attempting automated processing of paper acceptance...
๐ฆ๐ฆ๐ฆ ๐ Tweet for this paper ๐ ๐ฆ๐ฆ๐ฆ
๐จ๐จ๐จ THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! ๐จ๐จ๐จ
Here's what you must now do:
Party like you just published a paper! ๐๐๐ฆ๐๐ป๐ค
Any issues? notify your editorial technical team...
@arfon is there an issue with whedon right now? The PDF built fine for the accept PR, but it isn't appearing at https://doi.org/10.21105/joss.01531
@kyleniemeyer seems to appear for me fine on that link.
Weird! I still can't see it in Safari, but I can in Firefox. But the issue seems to be limited to me, so I'll say it's fine.
@ts-adi congrats on your article's publication in JOSS! Thanks to @dvberkel for reviewing and @gkthiruvathukal for editing!
:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:
If you would like to include a link to your paper from your README use the following code snippets:
Markdown:
[](https://doi.org/10.21105/joss.01531)
HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.01531">
<img src="http://joss.theoj.org/papers/10.21105/joss.01531/status.svg" alt="DOI badge" >
</a>
reStructuredText:
.. image:: http://joss.theoj.org/papers/10.21105/joss.01531/status.svg
:target: https://doi.org/10.21105/joss.01531
This is how it will look in your documentation:
We need your help!
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 for all your help with this @dvberkel @gkthiruvathukal @kyleniemeyer
A pleasure, @kyleniemeyer and @ts-adi. Big thanks to our reviewer, @dvberkel!
it was my pleasure. @ts-adi and team, you made awesome software