Restructure project folders to better accommodate (C++) unit tests

[uweseimet]

In order to use googletest for C++ it may make sense to restructure the current folder hierarchy. Instead of having sub-projects like web and raspberrypi inside each of src/ and test/, the sub-projects should be top-level and each contain src/ and test/. So instead of:

src/raspberrypi test/raspberrypi ...

we would have:

raspberrypi/src raspberrypi/test web/src web/test ...

or maybe better:

c++/src c++/test web/src web/test ...

or maybe that's even better:

c++/src c++/test python/src python/test ...

Further, sub-folders should be created for each resulting binary, at least for rasctl and rascsi, e.g.

c++/src/rascsi c++/src/rasctl c++/src/common ...

Or

c++/rascsi/src c++/rasctl/src c++/common/src ...

With common containing shared code.

The Makefile rules that assign object files to binaries also have to be changed, because the tests require a main() method of their own, which collides with rascsi::main().

[uweseimet]

@rdmark In case we agree on a structural change it should be done right after the 21.11 release. Even if nobody was willing to add unit tests (I experimented a bit with googletest, thus this ticket) the current structure is not that nice.

[rdmark]

@uweseimet Agreed that just after 21.11 is a good timing for making structural changes in the repo. It gives use time to weed out unexpected complications/bugs.

I like the idea of breaking it down by language. I suggest we introduce a c/ directory as well to accommodate the X68k native code:

c/RASCTL c/RASDRV c/RASETHER

For the Python packages, I'd like to do some research on the best test runner/framework for test automation before deciding on a final structure. We have effectively three Python packages: loopback_test, oled_monitor, and web. There is an opportunity to create common libraries for the two latter, where there is mild code duplication right now.

@akuker What are your thoughts here?

[uweseimet]

@rdmark I never looked at the x68k folder before. What is this code used for?

[rdmark]

@uweseimet They are code for native device drivers (RASDRV, RASETHER) and executables (RASCTL) for the Human68k OS, Sharp X68000. See wiki: https://github.com/akuker/RASCSI/wiki/X68000#X68000_Features

[akuker]

A minor thing - something about using the '+' character in a file path makes me cringe. I'd request that we use cpp instead of c++. I don't remember if that has bit me while working on makefiles or bash scripts or both in the past.....

What do you guys use for an IDE? Personally I use VSCode. TBH, I haven't dug into the unit testing capabilities of it, but it would be nice if it integrated nicely into an IDE.

A slight variation on what @uweseimet proposed....

rascsi/
   doc/ (nochange)
   hw/ (no change)
   cpp/
      librascsi/
         src/
         test/
      rascsi/
         src/
         test/
      rasctl/
         src/
         test/
      scsimon/
         src/
         test/
   python/
      oled/
         src/
         test/
      web/
         src/
         test/
      common/ ??
         src/
         test/
      loopback/ # I don't know if it makes sense to create tests for the loopback test.... it either works, or it doesn't :-)
         src/

[uweseimet]

@akuker I use Eclipse on Linux. Yes, "cpp" is better, indeed. Special characters have the potential for issues. "lib"or something similar instead of common is fine for me. For consistency we should try to use the same name for python as well.

[rdmark]

@akuker I use Notepad++ on Windows. Willing to change to a different IDE for UT integration. :)

[hsiboy]

Another Vs Code user here, this is a good call @uweseimet I too tried to put some simple tests together, but in the current state, its a real pain.

[uweseimet]

@hsiboy Would you agree that the structure proposed above (the latest one by @akuker) is a better approach, or would you suggest something else?

[hsiboy]

Yes, I think that @akuker approach is good as it will allow for growth/change in the future.

+1 from me.

[bzeiss]

@rdmark @akuker @uweseimet @hsiboy I have started restructuring the python scripts to a python folder in the branch bzeiss_restructure_project_folders_python. It seems to work with those changes.

I took the liberty to make two additional changes in the branch:

• I have renamed the oled service from monitor_rascsi to rascsi-oled. This has bugged me for a while the the systemd services weren't consistently named. They all start with rascsi or prefix rascsi now. I have adapted easy_install.sh with the migration work.
• I have made a few improvements to easy_install.sh along the way. The systemd stop/enable service commands sometimes caused error messages in my easy_install scripts depending on the actual state of the services. I have added some systemd checks around it so that services aren't stopped when they aren't started or not even enabled.

It would be great if someone could take an early look at how I moved everything and to check whether this is the right direction. I had to make some decisions along the way there haven't been part of the discussion here (e.g. where to move the oled image resources, systemd files etc.).

[rdmark]

@bzeiss I do think you're on the right track here! Good progress.

Some minor comments: if [ -d .git ]; then This check for a git repo will only work when in the root of the repo, won't it? It looks like you're doing this in various subdirs where it will always fail.

There have been talks about using LCD screens with RaSCSI, so I don't know if renaming the service to '-oled' is the right choice. But then again, perhaps we need a separate script for LCD screens anyways!

While doing cleanup, I'm wondering if renaming the start.sh scripts to something more descriptive might be a good idea too. E.g. start_rascsi_web.sh and start_rascsi_oled.sh etc.

Also, it'd be nice to rename 'rascsi_oled_monitor.py' to just 'oled_monitor.py' for consistency.

[bzeiss]

Good point, I will verify that .git check. It's quite possible that this is broken. I'm developing in Jetbrains IDEA where the contents of the local git content is synced to the raspberry via SSH duplication of the repository clone, so there is no .git directory. That made testing the scripts kind of annoying, so I just wanted to wrap the git stuff around a check so that the script still works when it's not a git cloned directory. But obvisouly that check must work. I'm testing now on the pushed branch as well and found already a few more things that are broken.

About the naming: yes, it would be best if we find a name that also matches the ongoing work with the control panel pcb. Maybe something like rascsi-control, rascsi-frontpanel or something along those lines. Based on that decision, we can decide on a start script naming.

[bzeiss]

@akuker @rdmark @uweseimet So I have been thinking about how to go forward. I'm currently working on three branches: one for the python restructuring, one for the ctrlboard and one for the proxy. The ctrlboard code is a complete reimplementation of everything, I think so far without any code from monitor_rascsi.

My proposition to move forward with all the changes is the following:

1) I'll rebase my python restructuring branch bzeiss_restructure_project_folders to the develop branch and create a pull request for it. This implies that the renaming of the current monitor_rascsi will still be rascsi-oled (see point 3 on why I propose this). This will essentially create the new structure as suggested above for only the Python-based content. This will give us some time to figure out whether it breaks anything that I haven't seen so far.

2) We can then start to work on the common python code directory. I have started with some parts of it in the ctrlboard branch. However, it would be good if we could brainstorm a few things regarding that. The current library code in src/web has some things in it like the flask internationalization where I'm not entirely sure how it can be shared to a non-webapp and whether it should be. Probably not a big thing, I just haven't dealt with it yet.

3) My suggestion is to keep the ctrlboard directory separate from the oled directory for a while. We can then carry the new controlboard feature as an experimental feature until a few people have a control board in their hands to play around and test it. At the same time, the existing oled feature is stable. I will then start to move over the necessary features from the oled to the ctrlboard project so that it covers both usage scenarios and the old oled python project can be removed when everything is good. In general, I'd intend to create a pull request for this as soon as the necessary parts exist in the shared code directory after the restructuring.

4) I haven't dealt with the restructuring of the C/C++ parts. I can generally do this as well, however, there is still a lot to do on my ctrlboard list. In general, I'd personally rather concentrate on making that part nice.

What do you guys think?

[rdmark]

@bzeiss I endorse your plan. Thanks for working on this!

As for i18n, the flask_babel library is an extension to Babel, which is a general-purpose Python i18n library, so it might be possible to reuse some components for other Python packages. It's worth experimenting with, I think. While the oled-monitor arguably don't need i18n, the ctrlboard may have a rich enough UI that it's worth considering, isn't it?

[bzeiss]

@rdmark Regarding the translation: yes, that's what I kind of hoped for as well. I'm not sure to what degree it is "clean" to integrate flask into general-purpose python code, but if the existing code can be reused or rephrased for general purpose i18n and pybabel as well so that it works for both scenarios, I'm definately suggesting we do it like that. And yes, I think the control board interface will definately gain from translations as well though the amount of characters is limited, so we'll have to see whether reusing strings for both web and control board ui will work well. I can imagine that we may need something like a "short phrases" mode.

[bzeiss]

Pull request for the python restructuring.

[uweseimet]

With the latest changes in https://github.com/akuker/RASCSI/pull/889 I would like to close this ticket. Moving the C++ tests to individual folders appeared to be fine, but in practice it is convenient to not have the tests split across several sub-folders. According to the initial suggestions the rascsi/rasctl-specific code is now located in separate folders.

Any objections to closing this ticket?

[rdmark]

@uweseimet I think it would be beneficial to make the very top level dir structure change, e.g.:

src/raspberrypi/ src/x68k/

to

cpp/ c/

It removes an unnecessary directory nesting, and signals clearly what to expect from each code base. Thoughts?

[uweseimet]

@rdmark Yes, you are right. Do you volunteer to do that? I have to admit that I am a bit tired of changes like this ;-). Maybe it's better to create a new, more precise ticket for that purpose.

[rdmark]

@uweseimet Sure I can volunteer to do that. It should be mostly a pretty route task I think.

[uweseimet]

@rdmark Working on this after my changes from the feature_merge_file_support branch have been merged would probably be a good point in time, because after this merge there are no other pending structural/filesystem changes.

[rdmark]

As per a discussion with @akuker I will take action on this once his Banana Pi feature is finished.

[uweseimet]

@rdmark I assume it will be part of the next release?

[rdmark]

@uweseimet I'm targeting the release following the next one.

[uweseimet]

@rdmark I suggest that you start after https://github.com/akuker/RASCSI/pull/915 has been merged, because this PR (targeted for the next release after 21.10) adds some new files. Changing folder structures before this merge might cause me inconvient merge conflicts. I will finalize https://github.com/akuker/RASCSI/pull/915 as soon as the develop branch is not frozen anymore, which I expect to be the case right after the 21.10 release.

[uweseimet]

@rdmark All pending changes that affect the fileystem have been merged, i.e. now it's a good time for structural changes.

[rdmark]

Sounds good. I'll work on this today.

[rdmark]

Addressed in https://github.com/akuker/RASCSI/pull/941