wb-toolbox
wb-toolbox copied to clipboard
Clarification on contributing process
I have a doubt on how to work (especially with multiple developers/contributors) on WB-Toolbox. The repo contains two Simulink models:
-
wb-toolbox/matlab/library/WBToolboxLibrary_repository.mdl
: This is the file that is meant to be modified by developers -
wb-toolbox/matlab/library/exported/WBToolboxLibrary.slx
: This is the file that is automatically generated by theexport_libraries
target, and it is exported in slx format, version 2014b .
My doubt is: in which format/version wb-toolbox/matlab/library/WBToolboxLibrary_repository.mdl
should be stored? Before https://github.com/robotology/wb-toolbox/pull/211/files it seem that it was in format 2019b, but the last PR changed it back to 2014b format.
Any idea @diegoferigo @nunoguedelha @CarlottaSartore @Giulero @gabrielenava ?
AFAIK, we never had a real guideline. Regading the mdl
, that is the version used to develop the library and from which the "exported" slx
is generated, was using whatever Matlab version the main developer had (for long time, mine xD). Since I was the only one accessing that file, I was not bothering to export the file for older versions.
This worked for long time due to how the maintainership was done, though now if we want we can discuss a more structured approach that allows everyone to contribute while providing some flexibility.
Ack, so we just need to agree the minimum version among the people modifying it (in the next future, I guess me, @nunoguedelha and @Yeshasvitvs for https://github.com/robotology/wb-toolbox/pull/178). Thanks!
Agreeing on the minimum version may not be enough, as the process of editing the wb-toolbox/matlab/library/WBToolboxLibrary_repository.mdl
file while keeping it at an old version is quite cumbersome, as you can't simply use the "save" option of Simulink, but you need to Export Model to Previous Version
, and you then need to use a different name for the file. If we all are on 2021a, probably we can just starting keeping the .mdl
model in that format, while continuing to export the .slx
in R2014b format?
The PR https://github.com/robotology/wb-toolbox/pull/209/files contains the wb-toolbox/matlab/library/WBToolboxLibrary_repository.mdl
model in R2021a format. If there are no objections, I would merge it as it is, and if future contributors want to use an earlier MATLAB, we can always export it back to the necessary version.
Related comment of @nunoguedelha in https://github.com/robotology/wb-toolbox/pull/209#issuecomment-847159822 :
@traversaro , I didn't have a chance to review the last version. It's ok because there are two other reviewers, but actually the model file
.../robotology-superbuild/src/WBToolbox/matlab/library/WBToolboxLibrary_repository.mdl
is still the version 2021b and I would have noticed that before the merge 😄 . Could you please convert it to 2019b? I don't know if you want to apply the policy you mentioned recently (support the last 4 MATLAB versions)?
That was indeed my doubt, but given that we already store a version compatible with old Simulink in wb-toolbox/matlab/library/exported/WBToolboxLibrary.slx
, what's the point of keeping another one wb-toolbox/matlab/library/WBToolboxLibrary_repository.mdl
?
Agreeing on the minimum version may not be enough, as the process of editing the
wb-toolbox/matlab/library/WBToolboxLibrary_repository.mdl
file while keeping it at an old version is quite cumbersome, as you can't simply use the "save" option of Simulink, but you need toExport Model to Previous Version
, and you then need to use a different name for the file. If we all are on 2021a, probably we can just starting keeping the.mdl
model in that format, while continuing to export the.slx
in R2014b format?
Not sure if @nunoguedelha has some input n this point.
Sorry, I do indeed. As far as I understood, the rational behind the existence of the two library formats .mdl
and .xls
was as follows: if a developer wants to change the library, he should change the .mdl
then export it to .xls
using the script export_library.m
. I guess this approach was to make sure that the library is always properly configured (EnableLBRepository
set to on
, version R2014b, etc).
I personally don't see the point in keeping the .mdl
at all, in any version, for two reasons:
- Probably the initial motivation to keep an
.mdl
file was to be able to merge it manually, relying on the text content, i.e. without the MATLAB dedicated tools (visdiff, etc). Considering the danger of merging this way, and our growing experience with the MATLAB diff/merge tools, I think this motivation has become obsolete. - Keeping the
.mdl
, specially in a more recent version than R2014b doesn't reduce the effort of exporting it to R2014b. And waiting that a user having MATLAB version < current.mdl
version, for istance R2019b < R2021b, needs it before doing the conversion, is problematic. He has to:- report the issue and ask someone (X) with MATLAB R2021b to kindly do the conversion.
- wait for that X to catch the request
- wait for X to be available for doing the conversion and push the changes.
Even if X is extremely responsive, I don't think that's a reliable way to proceed, and just increases the overall effort.
A simple way to go would be to
- Keep only the .xls (unless I missed some other motivation).
- change the script
export_library.m
, integrating the steps to export the library to version R2014b from the same input file (copy to a different name, load it, export to R2014b as the original name, close).
What do you think guys?
If it would be my choice, i.e. if I had to maintain the library, I'd probably keep the current structure. Working with a textual file is too handy in my opinion, and this could be even more helpful when there are many collaborators and many PR that have to merged together. Now, if this is a shared opinion among the current maintainers, I wouldn't have anything against moving towards a single slx library.
And just to double check, the reason why we don't have a single .mdl file is that images do not work with mdl files, if I remember correctly?
If I recall images were only part of the problem:
https://github.com/robotology/wb-toolbox/blob/4629ec927eafb0b0d8ff2cc7cbe2ef2f431c7cd6/matlab/export_library.m#L27-L28
We discussed on this with @nunoguedelha . We agreed that the process of manually exporting to R2014b (or any other version that is the one of the running MATLAB) is quite error prone, so just relying on editing and re-exporting wb-toolbox/matlab/library/exported/WBToolboxLibrary.slx
is not a desirable workflow. On the other hand, even if we keep wb-toolbox/matlab/library/WBToolboxLibrary_repository.mdl
in the repo to an old version, we would have exactly the same problem that we have with wb-toolbox/matlab/library/exported/WBToolboxLibrary.slx
, so that is not a solution as well, unless we all coordinate to use exactly the same MATLAB version, that may not be feasible in practice (even for conflicting requirements across projects and teams).
As a compromise, we reached the following agreement:
- We delete the
wb-toolbox/matlab/library/WBToolboxLibrary_repository.mdl
file from the committed repo, to have only a single source of truth given bywb-toolbox/matlab/library/exported/WBToolboxLibrary.slx
- We add a script (and associated build target)
import_library
, that creates on the fly thewb-toolbox/matlab/library/WBToolboxLibrary_repository.mdl
model for the version of MATLAB used by the contributor.
In this way, the contribution workflow remains similar to the existing one, with the only difference that the contributor needs to run import_library
as first step before contributing.