Raspberry Pi Add-Ons for FreeSSM

nikolaykm commented 6 years ago

Hello,

First of all thank you for developing FreeSSM and publishing its source code, its a great software!

In the recent months I was developing some new functionalities for FreeSSM and extending it under the https://github.com/nikolaykm/FreeSSM (forked repo) and https://github.com/nikolaykm/FreeSSMPlugins repos.

In short I've added a new layout skins for small display sizes that are making the FreeSSM usable from devices like RaspberryPi with attached small screen (in my case 3.5' screen size). Also I've extended its source base a little bit as a concept of proof so I can use it as simple reporting tool of preconfigured MBs and SWs. In that way I can connect to it with some external plugins and listen for a specific values and trigger some actions when the values change. As an example I created 3 plugins:

Rear camera view
Front and rear camera dashboard recorder
Better monitoring tool of some preconfigured MBs and SWs when driving using the RPi display

I've presented these ideas and the things that I've done recently in the OpenFest 2017 open source conference held in Bulgaria in the beginning of Nov 2017.

Here you can check the presentation that I made: https://www.youtube.com/watch?v=_su7msOyXMk&index=7&list=PLzUxAJX9n5vod0Ds8PyT-JG-FZQ317tZD

Do you think its worth adding some of the new features to the official FreeSSM repo?

Best regards, Nikolay Marinov.

Comer352L commented 6 years ago

Hi Nikolay, thanks for your work, contributions are always welcome. I'm too busy at the moment but I'm going to review the changes ASAP. Maybe I can simply pull/merge them as is. Thanks for your patience !

nikolaykm commented 6 years ago

Hi Comer352L,

Thank you too!

Yes I think you could merge them easily. I've tried to preserve the old code and added the new one with conditional compilation flags.

I'm still having some additional work for polishing the new changes, but in general the UI ones are almost done. However I'm also busy at the moment and probably the final polishing will happen some time next year.

Best regards, Nikolay.

aegis1980 commented 6 years ago

Hi Nicolay, Watched your video - looks good. I don't suppose you could commit your modifications (UI and TCP) to your fork could you, whatever state they are in? Delving into FreeSSM code to implement something similar myself is well beyond my capabilities: Using your mods to get at FreeSSM data to make some pretty plugins a little more within my coding ability! Jon

Comer352L commented 6 years ago

Thanks for your patience ! I have a free week now and just started reviewing/testing your changes.

Comer352L commented 6 years ago

Hmm... how is compilation with SMALL_RESOLUTION supposed to be invoked ? I can do something like qmake "CONFIG+=SMALL_RESOLUTION" "DEFINES+=SMALL_RESOLUTION" FreeSSM.pro make Not very comfortable... Is there a simpler method ?

Comer352L commented 6 years ago

Compilation is broken. :-( ... g++ -c -pipe -fno-gcse -std=c++11 -O2 -Wall -W -D_REENTRANT -fPIC -DTIXML_USE_STL -DQT_DISABLE_DEPRECATED_BEFORE=0x040000 -DQT_NO_DEBUG -DQT_PRINTSUPPORT_LIB -DQT_WIDGETS_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_CORE_LIB -I. -I. -Isrc -Isrc/tinyxml -Isrc/linux -isystem /usr/include/qt5 -isystem /usr/include/qt5/QtPrintSupport -isystem /usr/include/qt5/QtWidgets -isystem /usr/include/qt5/QtGui -isystem /usr/include/qt5/QtNetwork -isystem /usr/include/qt5/QtCore -Irelease -isystem /usr/include/libdrm -I. -I/usr/lib/qt5/mkspecs/linux-g++ -o release/MBsSWsListeners.o src/MBsSWsListeners.cpp src/MBsSWsListeners.cpp: In member function ‘void MBsSWsListeners::acceptNewConnection()’: src/MBsSWsListeners.cpp:27:20: error: variable ‘QDataStream out’ has initializer but incomplete type QDataStream out(&block, QIODevice::WriteOnly); ^ src/MBsSWsListeners.cpp:28:33: error: incomplete type ‘QDataStream’ used in nested name specifier out.setVersion(QDataStream::Qt_4_0); ^~ src/MBsSWsListeners.cpp: In member function ‘void MBsSWsListeners::publishData(const string&)’: src/MBsSWsListeners.cpp:52:24: error: variable ‘QDataStream out’ has initializer but incomplete type QDataStream out(&block, QIODevice::WriteOnly); ^

Comer352L commented 6 years ago

Ok, looks like you were using Qt4, right ? ;-) That works.

nikolaykm commented 6 years ago

Hi Comer352L,

Sorry for not replying earlier. I'm pretty busy these days.

I just saw your messages. Yes I was using Qt4. I think I didn't tested with Qt5 :)

I'll be monitoring thread more frequently so I can reply you shortly if you have more questions or issues with the features.

Regards, Nikolay.

Comer352L commented 6 years ago

Yes I was using Qt4. I think I didn't tested with Qt5 :)

No problem, I have tested it now. :) Compilation can be fixed easily by adding a #include <QDataStream> to the top of MBsSWsListeners.cpp.

Comer352L commented 6 years ago

Question: In some places (see commits 5+18) you add calls to setWindowsFlags(Qt::Window) to the dialogs. Does this really make any difference for you or did you just add them because we already have them in a few other places ? ;) On my systems, none of these calls (including the ones that already exist) has any notable effect. According to comments in the code, they might make a difference at least on Gnome 3. However, AFAIU the Qt documentation Qt::Dialog (the default setting) can suppress the minimize/maximize buttons of the window, which should be exactly what you want in fullscreen mode !?

Comer352L commented 6 years ago

Ok, I've finished reviewing/testing the first 19 commits (which add the UI for small resolutions). I would like to make a break here and get this stuff in before continuing.

In general I'm fine with your work. The approach makes sense and the UI looks good. :) But I would like to ask you to rebuild the commits incorporating some changes/fixes/improvements. Does that sound reasonable ?

nikolaykm commented 6 years ago

As I remember I've added the setWindowsFlags(Qt::Window) to the dialogs because of some bug that was preventing the dialog to really show in full screen. I think I observed it both on my Ubuntu Desktop system as well on the Raspbian distribution installed on my RPi.

I found some Stack Overflow threads suggesting for setting the dialog windows flags to Qt::Window: https://stackoverflow.com/questions/12645880/fullscreen-for-qdialog-from-within-mainwindow-only-working-sometimes

After that I didn't have these problems :)

Sure that sounds great! I also wanted to squash and rebuild the commits a little, but I didn't have the time. Can you compile a list of the changes/fixes/improvements and I'll incorporate them :)

Comer352L commented 6 years ago

What happens if you keep calling the setupUiFonts() methods ? I assume With VGA on a 3.5" display the fonts will be too small ? The story behind this is, that the famous OS from Redmond doesn't scale the fonts correctly in point size mode on screens with unusual dpis. :/ And the Qt-Designer isn't capable of specifying the font size in pixels.

nikolaykm commented 6 years ago

Yes I had problems controlling the font size when calling the setupUiFonts, but that was not the only problem.

Because of the small display size I didn't had enough space on the screen for some information, so I needed to not include all labels and some values that are present in the original version and setupUiFonts was expecting their controls to be available, so I've decided to just conditionally exclude that part of the code for the small display version for the moment.

Comer352L commented 6 years ago

For the records: I have converted the software from using pixel sizes to point sizes for the fonts. That has simplified the code lot and gives the OS more control over font scaling, which seems to work on Windows, too, these days.

I have put the result into a separate new branch called "ui", because 1) there are currently at least 2 bugs in the Qt5 library that mess up font parameter inheritance. See https://bugreports.qt.io/browse/QTBUG-66136 If the Qt library doesn't get fixed in the near future, I will apply additional workarounds before merging the changes into the "master" branch. 2) although the situation on Windows has improved a lot, problems should still be expected with small resolutions (<1024x768) on screens with unusal dpi values. I can't test this at the moment. For those who can and are interested in this use case - feedback is appreciated.

Comer352L commented 6 years ago

Ok, here is the list of changes I would like you to make:

0) base your work on the new "ui" branch 1) Fix the indentions: use tabs instead of whitespaces, #ifdef always start at the beginning of the line 2) some commits break things or apply changes, which are then fixed and/or reverted later again. Add new things properly right from the beginning, which means

merge commits 3 and 19 in 1
merge commits 4 and 6 in 2 ... 3) I suggest splitting
commit 15 into 3 parts (DCs, adjustments, systests)
commit 18 into 2 parts (actuators, about) 4) setupUiFonts() are now gone, which makes lots of #ifdefs futile 5) Don't touch the original ui files (normal ui). 6) Improve the compilation for small screen resolutions: in FreeSSM.pro
rename the config section "SMALL_RESOLUTION" to "small-resolution" (small letters are common for CONFIGs and it avoids confusion with the corresponding #DEFINE)
add DEFINES += SMALL_RESOLUTION to the small-resolution configuration, which will cause compilation to be invoked with -DSMALL_RESOLUTION automatically. Now all you have to do is to append "CONFIG+=small-resolution" to the qmake call.
for those who want to modify the build permanently, it might make sense to add a line CONFIG+=small-resolution to the project file and comment it out by default. 7) Document the compilation for small resolutions in README.txt 8) You forgot to remove the "print" button from CUcontent_DCs_stopCodes, too. ;) 9) Window flags tweaking: I'm pretty sure they are not supposed to be touched and there is a bug in the Gnome/Unity/Gtk world. But they seem to cause no trouble on other platforms, so we can live with this workaround for now. But please add small comments explaining what exactly they are needed for, so that we later know how to reproduce the issues. 10) feel free to add yourself to the list of contributors (About.ui)

The send me a pull request. Don't worry, if something remains, I can fix it up myself afterwards. Thanks !

nikolaykm commented 6 years ago

Thanks for the detailed list! Many of the things in it were in my radar too, but it I didn't have the patience and time to compile them in a TODO list :)

I hope I'll have some time in the end of the month and will refactor the code and rebase it over the ui branch.

nikolaykm commented 6 years ago

Ok, I think all the items from the above list are now covered and tested, so I'm creating a pull request :)

Comer352L commented 6 years ago

I have started working on command line start-up parameters as suggested by Nikolay. It requires a few strcutural changes in the control unit dialogs which are on my ToDo list since a long time. 80% of the work is done, but I will have to make a break now for the next 1-2 weeks.

I've decided for a more generalized apporach, basically something like

./FreeSSM -c engine -f mbsw -p file=/aaa/mymbswselection.list autostart

I will also use the existing MB/SW list file functionality.

Comer352L / FreeSSM

Raspberry Pi Add-Ons for FreeSSM #19