Clean main and init files from different architectures

[cochicde]

Duplicated code was removed and a centralized entry point to forte is given (when possible).

[franz-hoepfinger-4diac]

i test the FreeRTOS now.

[franz-hoepfinger-4diac]

so, Compilation fails: https://gist.github.com/franz-hoepfinger-4diac/108f6b91c1113aae71754145c77b8276

undefined reference to 'forteJoinInstance'

now i changed my forte Component in ESP32 to this: https://gitlab.com/meisterschulen-am-ostbahnhof-munchen/esp32-4diac-example-application-no-lua/-/tree/develop/Application/components/forte_main?ref_type=heads

and it compiles again.

i will now test if it is working.

[cochicde]

so, Compilation fails: https://gist.github.com/franz-hoepfinger-4diac/108f6b91c1113aae71754145c77b8276

I changed the interface to reflect properly what the functions are doing and more closely related to the Device functions. Also, ideally you should be able now to have everything you need by including the file src/arch/forte.h and src/common/devlog.h

undefined reference toforteJoinInstance' ->forteJoinInstanceis nowforteWaitForInstanceToStop. A possible implementation ofmain` for freeRTOS is here https://github.com/eclipse-4diac/4diac-forte/blob/853c75649900ea3c9c4305adc88b050618ce2e55/src/arch/freeRTOS/main.cpp

[diplfranzhoepfinger]

undefined reference to forteJoinInstance' ->forteJoinInstanceis nowforteWaitForInstanceToStop. A possible implementation of main` for freeRTOS is here https://github.com/eclipse-4diac/4diac-forte/blob/853c75649900ea3c9c4305adc88b050618ce2e55/src/arch/freeRTOS/main.cpp

which is nice, but not so practical. 

https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/system/freertos.html#application-entry-point 

main is not the App Entry Point, but app_main but also this is normally not true for 4diac, as you might want to start things like Network Stack, Wifi, Webserver etc.. before.

[cochicde]

undefined reference to forteJoinInstance' ->forteJoinInstanceis nowforteWaitForInstanceToStop. A possible implementation of main` for freeRTOS is here https://github.com/eclipse-4diac/4diac-forte/blob/853c75649900ea3c9c4305adc88b050618ce2e55/src/arch/freeRTOS/main.cpp

which is nice, but not so practical. 

https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/system/freertos.html#application-entry-point 

main is not the App Entry Point, but app_main but also this is normally not true for 4diac, as you might want to start things like Network Stack, Wifi, Webserver etc.. before.

I see, but the app_main is something specific to ESP-IDF and not something generic to freeRTOS. The main.cpp file here in forte is mostly for guidance purposes, as you said other things might need to be done before and each application might do it differently.

The way you use it hasn't change much, just the signatures of the methods and the includes should be different now.

[kaartee]

Just letting you know that I wouldn't be able to review this PR with regard to the Zephyr port in the near future. I'm sorry.

[diplfranzhoepfinger]

I see, but the app_main is something specific to ESP-IDF and not something generic to freeRTOS. 

wrong. 

it is vanilla FreeRTOS:

https://github.com/FreeRTOS/FreeRTOS/blob/bf046c15f756ac204071bdac04637a3a9d5344ed/FreeRTOS/Demo/CORTEX_M0%2B_NUCLEO_L010RB_GCC_IAR/Demo/app_main.c#L51

The main.cpp file here in forte is mostly for guidance purposes, as you said other things might need to be done before and each application might do it differently.

true

The way you use it hasn't change much, just the signatures of the methods and the includes should be different now.

[kaartee]

To compile with a C (not C++) main source file:

src/arch/forte.h:

@@ -38,7 +38,7 @@ extern "C" {
   * @param paResultInstance Address  to store the created forte instance
   * @return FORTE_OK if no error occurred, other values otherwise
   */
-  FORTE_SHARED_PREFIX FORTE_STATUS forteStartInstance(unsigned int paPort, TForteInstance* paResultInstance);
+  FORTE_SHARED_PREFIX enum FORTE_STATUS forteStartInstance(unsigned int paPort, TForteInstance* paResultInstance);

  /**
   * \brief Start forte instance with posibilities of more arguments
@@ -47,7 +47,7 @@ extern "C" {
   * @param paResultInstance Address  to store the created forte instance
   * @return FORTE_OK if no error occurred, other values otherwise
   */
-  FORTE_SHARED_PREFIX FORTE_STATUS forteStartInstanceGeneric(int argc, char *argv[], TForteInstance* paResultInstance);
+  FORTE_SHARED_PREFIX enum FORTE_STATUS forteStartInstanceGeneric(int argc, char *argv[], TForteInstance* paResultInstance);

  /**
   * \brief Request termination of a Forte instance

[kaartee]

With above change, this PR builds for Zephyr. I haven't started or tested it any other way yet, though.

[cochicde]

With above change, this PR builds for Zephyr. I haven't started or tested it any other way yet, though.

thanks! that's was helpful. I added a typedef to fix that issue

[cochicde]

@azoitl this is ready now

[cochicde]

I think the startuphook.h should not be moved. The rest look ok. Although at a first glance a bit complicated.

I saw that startupHook.h (and its cpp.in) don't have anything related to the arch folder. I see the arch folder as the place for code that may vary for each platform, so I didn't see the any difference from forteinit.cpp.in or stringlist.cpp.in which are located directly under src.

I see the complication too. Maybe I was too greedy to try to make it really platform abstract that it became complex. One thing I see that is too much for example is the main_helper. My problem was that windows doesn't have one of the three signals we're using for all other cases. We could just have a #ifndef WIN32 or similar and include that signal in a simpler main.cpp file. what do you think?

[azoitl]

I think the startuphook.h should not be moved. The rest look ok. Although at a first glance a bit complicated.

I saw that startupHook.h (and its cpp.in) don't have anything related to the arch folder. I see the arch folder as the place for code that may vary for each platform, so I didn't see the any difference from forteinit.cpp.in or stringlist.cpp.in which are located directly under src.

the startuphook is and shall only be used in architecture. Thats the reason I originally put the h file there. code generation is correcter and easier in the main src directory therefore I kept it there. placing the .h file in src may imply others can or shall use it. Which I find a bit missleading.

I see the complication too. Maybe I was too greedy to try to make it really platform abstract that it became complex. One thing I see that is too much for example is the main_helper. My problem was that windows doesn't have one of the three signals we're using for all other cases. We could just have a #ifndef WIN32 or similar and include that signal in a simpler main.cpp file. what do you think?

Originally I thought it was the way how github presented your changes therefore I fetched them to look at them locally. However it didn't get better. To understand how your new code works I have to look to at least 8 files, maybe more. A 4diac FORTE internal cpp file now uses very strange and abstract C functions instead of a more C++ style. And even there stuff is separated into different parts that I think may not be necessary.

For me the C functions are there to allow to create a 4diac FORTE instance from C code, e.g., when using it as a lib or linking it to an small os.

The endhook is and never was platform independent. It was only for posix platforms. Here one size fits all is definitely wrong.

However, I think you did here is an important work to make the 4diac platform specific start-up code less duplicated and better maintainable. But we are not fully there yet.

[cochicde]

Just organizing the points for easy discussing them:

1. startuphook: I see your point in the sense that no one else should use it. I'll move it back
2. endhook: this is also being used by the windows platform, that's the reason I tried to abstract it. We could have one main for each posix and another for windows, but they are so similar and therefore we end up with duplicated code. I'm not sure what to do with it

3. C vs C++ inteface: You're right, I think I pushed it too far. My main idea (pun intended) was to have the main also test the C interface, but I could move this C-style main into the test folder, basically in order to test this interface. I see two problems still regarding this:

   • extra code will be compiled: as this PR is now, forte as a library offers the C interface basically so anyone can use it (meaning C compilers can work with it). To offer this interface, the "implementation" of this interface (forte.cpp now) where all the C style creation of forte happens, uses the C4diacFORTEInstance class "bridging" to C++. If we have the main in a C++ style, basically as it is outside this PR, then it looks very similar as the forte.cpp which is then not used and you endup with unused code (which is not that bad).

   While writing this message, I thought about the following: we could have a forte library which includes all until C4diacFORTEInstance and none of the C interface. Then, another forte_c library (maybe under a CMake parameter to enable it) which basically adds these C-interface file and links to forte. The main in arch in C++ style would then link to the forte library, and a main in the test folder would test the forte_c library.

   • forte.h name: the naming of this file can be confusing. I thought forte_c_interface.h for clarity, what do you think?

[azoitl]

ad 2) me neither

ad 3) your last proposal sounds quite good. I also have no problem that not all code is used in all cases. There are different options for different use-cases. And depending on what I need I would pull in the right one.

[cochicde]

Just organizing the points for easy discussing them:

1. startuphook: I see your point in the sense that no one else should use it. I'll move it back
2. endhook: this is also being used by the windows platform, that's the reason I tried to abstract it. We could have one main for each posix and another for windows, but they are so similar and therefore we end up with duplicated code. I'm not sure what to do with it

3. C vs C++ inteface: You're right, I think I pushed it too far. My main idea (pun intended) was to have the main also test the C interface, but I could move this C-style main into the test folder, basically in order to test this interface. I see two problems still regarding this:

   • extra code will be compiled: as this PR is now, forte as a library offers the C interface basically so anyone can use it (meaning C compilers can work with it). To offer this interface, the "implementation" of this interface (forte.cpp now) where all the C style creation of forte happens, uses the C4diacFORTEInstance class "bridging" to C++. If we have the main in a C++ style, basically as it is outside this PR, then it looks very similar as the forte.cpp which is then not used and you endup with unused code (which is not that bad).

   While writing this message, I thought about the following: we could have a forte library which includes all until C4diacFORTEInstance and none of the C interface. Then, another forte_c library (maybe under a CMake parameter to enable it) which basically adds these C-interface file and links to forte. The main in arch in C++ style would then link to the forte library, and a main in the test folder would test the forte_c library.

   • forte.h name: the naming of this file can be confusing. I thought forte_c_interface.h for clarity, what do you think?

I implemented the things presented here:

1. startuphook was moved back where it was under arch
2. main.cpp is using c++ style code as it was before using C4diacFORTEInstance.
   • I removed all the complex stuff surrounding endhook and now everything is back in main.cpp and for the special signal in posix, I just have a special #ifndef WIN32
   • Some PLCNext startup code was moved into the PLCNext module using the startuphook
   • forteinit was moved into the architecture initialization
   • Some windows code in fdhandler was moved into the architecture initialization of windows.
3. C Interface:
   • The C interface code was moved to arch/c_interface and are included only on demand via cmake.
   • They are linked to FORTE_LITE and create a static and/or shared library depending on cmake variables
   • Tests were added to make sure that an executable can link to these new libraries and perform some test on the interface itself
   • Similar to this, "compilation" tests were added to make sure that the static and shared libraries of forte can be linked to (not related to the c-interface libraries)
4. Re-organization of target preparation in src/CMakeListst.txt:
   • First, all the files to compile and configurations are figured out, and only then the targets are prepared.
   • For the target preparation, FORTE_LITE is used as a based target where all confurations are set (compilation flags, link flags, libraries, directories, ....).
   • All other targets (forte exectuable, forte static/shared libraries, c interface static/shared libraries) link to FORTE_LITE and doing that, they "inherit" all the configurations from it.

Things not resolved:

• in windows, the shared libraries are different and I could not make them work properly for some reason, so I excluded for now the tests for it.
• The c-interface shared library: I wanted to keep the exported symbols only limited to the C interface to make its loading time shorter, but still all symbols are exported. It's not a show stopper.

[cochicde]

tests are failing because of this https://github.com/eclipse-4diac/4diac-forte/commit/2a0a3e410a47cbda302b5010f6e146e851e947d2 which should be fixed here #195

[azoitl]

tests are failing because of this 2a0a3e4 which should be fixed here #195

I merged the fix back could you please rebase sot hat we can have the unit test chekcing your pr.