Improvements to OpenVarioMenu #1214

Scumi · 2023-05-09T10:47:03Z

Brief summary of the changes

Work-in-progress implementation update for the OpenVarioMenu.

Related issues and discussions

#1107
#1207
Openvario/meta-openvario#199

src/OV/System.hpp

src/OV/System.cpp

src/OV/System.hpp

src/OV/System.cpp

src/OV/System.hpp

MaxKellermann · 2023-05-12T08:39:27Z

src/OV/SystemDialog.cpp:111:28: error: lambda capture 'this' is not used [-Werror,-Wunused-lambda-capture]

Scumi · 2023-05-12T09:50:48Z

src/OV/SystemDialog.cpp:111:28: error: lambda capture 'this' is not used [-Werror,-Wunused-lambda-capture]

Did not observe this during make build with gcc, but could re-produce with clang. Fixed.

MaxKellermann · 2023-05-12T10:22:12Z

More compiler complaints:

In file included from ./src/io/BufferedOutputStream.hxx:11,
                 from src/OV/System.cpp:10:
In constructor ‘fmt::v9::format_int::format_int(int)’,
    inlined from ‘void OpenvarioSetRotation(DisplayOrientation)’ at src/OV/System.cpp:112:41:
/usr/include/fmt/format.h:3674:54: error: ‘rotation’ may be used uninitialized [-Werror=maybe-uninitialized]
 3674 |   explicit format_int(int value) : str_(format_signed(value)) {}
      |                                         ~~~~~~~~~~~~~^~~~~~~
src/OV/System.cpp: In function ‘void OpenvarioSetRotation(DisplayOrientation)’:
src/OV/System.cpp:92:7: note: ‘rotation’ was declared here
   92 |   int rotation;
      |       ^~~~~~~~

MaxKellermann · 2023-05-12T10:23:25Z

src/OV/System.cpp

+void
+OpenvarioSetLanguage(const char* lang)
+{
+  Run("localectl", "set-locale", lang);


Did you test this? Does this work?

No, not tested yet. I should've marked that commit as WIP as well. Was just an idea as a first shot without further looking into it. I can keep such things out of this Draft PR if it wastes your time looking at it.

MaxKellermann · 2023-05-12T10:24:15Z

src/OV/System.cpp

+OpenvarioSetLanguage(const char* lang)
+{
+  Run("localectl", "set-locale", lang);
+  Run("export", fmt::format("LANG={}", lang).c_str());


This has zero effect, even if this function call were able to invoke a shell built-in (which it does not). So you spawn a shell, use export to set an environment variable in this process, and then the process exits. The environment change is gone.

Scumi · 2023-05-12T10:45:41Z

More compiler complaints:

[...]

May I ask what your compile command is?
I'm wondering why I do not see those compiler complaints.

Also you merged the rotation commit as is, right? I need to issue a fixup commit to have this variable initialized properly.

The screen was rotated the wrong way after restart for both portrait orientations.

MaxKellermann · 2023-05-15T15:19:09Z

src/OV/OpenVarioMenu.cpp

+  /*the x-menu is waiting a second to solve timing problem with display rotation */
+  std::this_thread::sleep_for(std::chrono::seconds(1));


This sounds really horrible, and the justification for such a horrible kludge must be documented really well. Just saying "timing problem" isn't enough.