Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean main and init files from different architectures #151

Merged
merged 48 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
0e55f4b
fix new device interface in ctfTracer test
cochicde Jun 5, 2024
8dbe5b6
first part
cochicde Jun 6, 2024
7095b0f
cleanup all archs
cochicde Jun 6, 2024
4ca64a3
Merge remote-tracking branch 'origin/develop' into clean-main
cochicde Jun 6, 2024
924e58d
fix merge conflict
cochicde Jun 6, 2024
9f8a794
add missing flag for shared library
cochicde Jun 7, 2024
242f3dc
fix windows architecture declaration and cleanup startuphook file
cochicde Jun 7, 2024
ea89b0d
Update src/arch/forte.cpp
cochicde Jun 8, 2024
581f023
Update src/arch/forte.cpp
cochicde Jun 8, 2024
ccf943e
Apply suggestions from code review
cochicde Jun 8, 2024
fe0ab41
some more cleanup
cochicde Jun 8, 2024
c885559
Merge branch 'clean-main' of github.com:cochicde/4diac-forte into cle…
cochicde Jun 8, 2024
cef37e2
fix all comments
cochicde Jun 8, 2024
7f82c8a
fix all
cochicde Jun 8, 2024
a6c0299
limit shared library test only when forte is build as shared library
cochicde Jun 8, 2024
ed7c92c
move signals hook to each arch
cochicde Jun 8, 2024
853c756
use devlog in freeRTOS
cochicde Jun 8, 2024
df4aef8
add typedef when compiling as C
cochicde Jun 14, 2024
0bc7d6f
Merge remote-tracking branch 'origin/develop' into clean-main
cochicde Jun 16, 2024
d547ea7
separate c interface library and add test for it
cochicde Jul 12, 2024
ae4b5a6
Merge remote-tracking branch 'origin/develop' into clean-main
cochicde Jul 12, 2024
8b4511d
clean freertos main
cochicde Jul 12, 2024
21acd20
try fix on windows
cochicde Jul 12, 2024
9056c2b
try windows fix 2
cochicde Jul 12, 2024
5dc7fac
do some more cleanup
cochicde Jul 13, 2024
5baf266
last cleanup, promise
cochicde Jul 13, 2024
c026dab
build libraries in github workflows
cochicde Jul 13, 2024
52faabd
link boost library only in posix
cochicde Jul 13, 2024
f33a6b0
add linker option also to new tests
cochicde Jul 13, 2024
a0b71ba
fix boost include
cochicde Jul 13, 2024
35e0c58
export windows symbols for shared library
cochicde Jul 13, 2024
21700b2
try fixing windows issue
cochicde Jul 13, 2024
e7a14ef
export windows symbols at a global level
cochicde Jul 14, 2024
986aef3
disable shared library test in windows
cochicde Jul 14, 2024
cd563e8
update headers
cochicde Jul 23, 2024
b27df14
Merge remote-tracking branch 'origin/develop' into clean-main
cochicde Jul 24, 2024
1f9ebd8
Merge remote-tracking branch 'origin/develop' into clean-main
cochicde Jul 26, 2024
6e55f1f
add delay in tests
cochicde Jul 27, 2024
44ea506
fixes after feedback
cochicde Aug 13, 2024
a41dc27
remove template parameter from general architecture
cochicde Aug 14, 2024
fd47279
use BOOST_TEST where possible
cochicde Aug 15, 2024
9ea8b84
use helper function to print response type
cochicde Aug 15, 2024
a4a624b
fix linker flags
cochicde Sep 7, 2024
663b0d7
fix linker flags properly
cochicde Sep 7, 2024
0c669d0
Merge remote-tracking branch 'origin/develop' into clean-main
cochicde Sep 7, 2024
29dca66
make linker libraries for C library PUBLIC
cochicde Sep 7, 2024
45cd92d
remove link to shared boost
cochicde Sep 8, 2024
0e9f85f
use unnamed namespace in freeRTOS main
cochicde Sep 15, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/cmake-multi-platform.yml
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ jobs:
-DFORTE_MODULE_RT_Events=ON
-DFORTE_MODULE_SIGNALPROCESSING=ON
-DFORTE_MODULE_UTILS=ON
-DFORTE_BUILD_STATIC_LIBRARY=ON
-DFORTE_BUILD_SHARED_LIBRARY=ON
-DFORTE_C_INTERFACE=ON
-DFORTE_TESTS=ON
${{ matrix.os == 'windows-latest' && format('-DFORTE_TESTS_INC_DIRS={0}/boost.1.84.0/lib/native/include', github.workspace) || '' }}

Expand Down
4 changes: 4 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# Copyright (c) 2010 - 2015 Profactor GmbH, AIT, fortiss GmbH
# 2010-2015, 2020 TU Wien/ACIN
# 2022 Martin Erich Jobst
# 2024 Jose Cabral
#
# This program and the accompanying materials are made available under the
# terms of the Eclipse Public License 2.0 which is available at
Expand Down Expand Up @@ -62,6 +63,9 @@ mark_as_advanced(FORTE_BUILD_STATIC_LIBRARY)
set(FORTE_BUILD_SHARED_LIBRARY OFF CACHE BOOL "Build FORTE as shared library")
mark_as_advanced(FORTE_BUILD_SHARED_LIBRARY)

SET(FORTE_C_INTERFACE OFF CACHE BOOL "Build C interface to Forte")
mark_as_advanced(FORTE_C_INTERFACE)

#######################################################################################
# Determine the loglevel
#######################################################################################
Expand Down
290 changes: 182 additions & 108 deletions src/CMakeLists.txt

Large diffs are not rendered by default.

6 changes: 2 additions & 4 deletions src/arch/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#*******************************************************************************
# Copyright (c) 2010 - 2018 ACIN
# Copyright (c) 2010 - 2024 ACIN, Jose Cabral
# This program and the accompanying materials are made available under the
# terms of the Eclipse Public License 2.0 which is available at
# http://www.eclipse.org/legal/epl-2.0.
Expand Down Expand Up @@ -64,6 +64,4 @@ if (FORTE_STACKTRACE)
endif ()
endif ()

forte_add_sourcefile_with_path_cpp(${CMAKE_BINARY_DIR}/src_gen/startuphook.cpp) # created file


forte_add_sourcefile_with_path_cpp(${CMAKE_BINARY_DIR}/src_gen/startuphook.cpp)
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2018 fortiss GmbH
* Copyright (c) 2018 - 2024 fortiss GmbH, Jose Cabral
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
Expand All @@ -11,42 +11,38 @@
* Tarik Terzimehic - make OPC UA server port setable from the command line
*******************************************************************************/

#include "forte_instance.h"
#include "forteinstance.h"
#include <forte_printer.h>

#include "fortenew.h"
#include "forte_architecture.h"
#include <stdio.h>
#include <stdlib.h>
#include "../../stdfblib/ita/RMT_DEV.h"
#include "forte_c.h"

#include "../utils/mainparam_utils.h"
#include "forte_architecture.h"
#include "forte_printer.h"
#include "forteinstance.h"

unsigned const int cgForteDefaultPort = 61499;
static const unsigned int scForteDefaultPort = 61499;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the coding guidelines doesn't know the prefix sc for static const, only cg for const global.

While file-scope globals theoretically have less impact, they are only an extern keyword, used by someone else, away being visible in other compilation units as well.

If we want to code the difference of usage into the name, I would rather suggest cf for "constant file" instead of
sg "static const", as static const doesn't imply anything than the variable is some sort of global variable and lives in this compilation unit. "cf" would also be more in line with "cg" for const global,

i will mark this only here, to not split up the discussion on several review comments, but my comment is valid for all "sc" changes

@azoitl @mx990 additional opinions are appreciated

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also recommend to add the "U" suffix to the number, as its an unsigned constant

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would strongly suggest to use an unnamed namespace in C++ instead of static linkage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went with the unnamed namespace since it's more c++ style. Regarding the naming convetion, since there's no defined convention for constant variables in unnames namespaces I didn't add any prefix to avoid having a "convention" only in that file which is not documented anywhere

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the unnamed namespace solution. Perhaps it wasn't that clear in my comment, if we feel we need another prefix to show the intent/usage we could add one to the coding guidelines.
I wasn't suggesting using another undocumented prefix in favor of another

static const unsigned int scMaxPortValue = 65535;

/*!\brief Check if the correct endianess has been configured.
*
* If the right endianess is not set this function will end FORTE.
*/
/**
* @brief Check if the correct endianess has been configured.
*/
bool checkEndianess();

void forteGlobalInitialize(){
CForteArchitecture::initialize();
int forteGlobalInitialize(int argc, char *argv[]){
return CForteArchitecture::initialize(argc, argv);
}

void forteGlobalDeinitialize(){
CForteArchitecture::deinitialize();
int forteGlobalDeinitialize(){
return CForteArchitecture::deinitialize();
}

int forteStartInstance(unsigned int paPort, TForteInstance* paResultInstance){
FORTE_STATUS forteStartInstance(unsigned int paPort, TForteInstance* paResultInstance){

if(65535 < paPort){
if(scMaxPortValue < paPort){
DEVLOG_ERROR("Provided port %d is not valid\n", paPort);
return FORTE_WRONG_PARAMETERS;
}

if(0 == paPort){
paPort = cgForteDefaultPort;
paPort = scForteDefaultPort;
}

char progName[] = "forte";
Expand All @@ -60,50 +56,59 @@ int forteStartInstance(unsigned int paPort, TForteInstance* paResultInstance){
return forteStartInstanceGeneric(3, arguments, paResultInstance);
}

int forteStartInstanceGeneric(int argc, char *arg[], TForteInstance* paResultInstance){

if(!CForteArchitecture::isInitialized()){
return FORTE_ARCHITECTURE_NOT_READY;
}
FORTE_STATUS forteStartInstanceGeneric(int argc, char *argv[], TForteInstance* paResultInstance){

if(0 == paResultDevice){
if(nullptr == paResultInstance){
DEVLOG_ERROR("Provided result instance parameter is not valid\n");
return FORTE_WRONG_PARAMETERS;
}

if(0 != *paResultDevice){
if(nullptr != *paResultInstance){
DEVLOG_ERROR("Provided result instance already started\n");
return FORTE_DEVICE_ALREADY_STARTED;
}

if(!checkEndianess()){
// logged already in the function
return FORTE_WRONG_ENDIANESS;
}

const char *pIpPort = parseCommandLineArguments(argc, arg);
if((0 != strlen(pIpPort)) && (nullptr != strchr(pIpPort, ':'))){
C4diacFORTEInstance *instance = new C4diacFORTEInstance();
if(!instance->startupNewDevice(ipPort)) {
delete instance;
return FORTE_COULD_NOT_CREATE_DEVICE;
}
*paResultInstance = instance;
DEVLOG_INFO("FORTE is up and running\n");
if(!CForteArchitecture::isInitialized()){
DEVLOG_ERROR("The low level platform should be initialized before starting a forte instance\n");
return FORTE_ARCHITECTURE_NOT_READY;
}
else{ //! If needed call listHelp() to list the help for FORTE

const auto ipPort = parseCommandLineArguments(argc, argv);
if((0 == strlen(ipPort)) || (nullptr == strchr(ipPort, ':'))){
listHelp();
return FORTE_WRONG_PARAMETERS;
}

C4diacFORTEInstance *instance = new C4diacFORTEInstance();
if(!instance->startupNewDevice(ipPort)) {
delete instance;
return FORTE_COULD_NOT_CREATE_DEVICE;
}
*paResultInstance = instance;

return FORTE_OK;
}

void forteStopInstance(int, TForteInstance paInstance){
if(!CForteArchitecture::isInitialized() || paResultInstance == nullptr){
void forteRequestStopInstance(TForteInstance paInstance){
if(!CForteArchitecture::isInitialized() || paInstance == nullptr){
return;
}
C4diacFORTEInstance *instance = static_cast<C4diacFORTEInstance *>(paResultInstance);
auto *instance = static_cast<C4diacFORTEInstance *>(paInstance);
instance->triggerDeviceShutdown();
}

void forteWaitForInstanceToStop(TForteInstance paInstance) {
if(!CForteArchitecture::isInitialized() || paInstance == nullptr){
return;
}
auto *instance = static_cast<C4diacFORTEInstance *>(paInstance);
instance->awaitDeviceShutdown();
delete instance;
DEVLOG_INFO("FORTE finished\n");
}

bool checkEndianess(){
Expand Down
82 changes: 82 additions & 0 deletions src/arch/c_interface/forte_c.h
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file does not seem to be architecture-dependent. Should this not rather live directly in src?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agrre, but before moving it I would leave that for discussion mainly because there was a similar case somewhere up there in the comments regarding the startuphook file

@azoitl

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currently we have everything that is used to start 4diac FORTE on a specific device in the arch folder. Therefore I would like to keep it here. Because when I'm porting 4diac FORTE to a new device I would look here first.

Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*******************************************************************************
* Copyright (c) 2018 - 2024 fortiss GmbH, Jose Cabral
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Jose Cabral - initial API and implementation and/or initial documentation
*******************************************************************************/

#ifndef SRC_ARCH_FORTE_C_H_
#define SRC_ARCH_FORTE_C_H_

#ifndef FORTE_SHARED_PREFIX
#define FORTE_SHARED_PREFIX
#endif

#ifdef __cplusplus
extern "C" {
#endif

typedef enum FORTE_STATUS{
FORTE_OK = 0,
FORTE_DEVICE_ALREADY_STARTED,
FORTE_WRONG_ENDIANESS,
FORTE_WRONG_PARAMETERS,
FORTE_ARCHITECTURE_NOT_READY,
FORTE_COULD_NOT_CREATE_DEVICE,
} FORTE_STATUS;

typedef void* TForteInstance;

/**
* \brief Start forte instance
* @param paPort The port on which forte will listen. Use 0 for default 61499
* @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);

/**
* \brief Start forte instance with posibilities of more arguments
* @param argc Number of arguments in arg
* @param argv Arguments
* @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);

/**
* \brief Request termination of a Forte instance
* @param paInstance Instance to request for termination
*/
FORTE_SHARED_PREFIX void forteRequestStopInstance(TForteInstance paInstance);

/**
* \brief Waits indefinitely for the intance to stop and deletes it
* @param paInstance Instance to terminate
*/
FORTE_SHARED_PREFIX void forteWaitForInstanceToStop(TForteInstance paInstance);

/**
* \brief Initializes the architecture. Prepare all resources needed by the Forte's instances. Must be called once before the first Forte instance is started
* @param argc Number of arguments in arg
* @param argv Arguments
* @return 0 if no error occurred, other values otherwise. Any error value is provided by the architecture and not by forte
*/
FORTE_SHARED_PREFIX int forteGlobalInitialize(int argc, char *argv[]);

/**
* \brief Deinitializes the architecture. Frees all resources used by Forte's instances. Must be called after the last instance is ended
* @return 0 if no error occurred, other values otherwise. Any error value is provided by the architecture and not by forte
*/
FORTE_SHARED_PREFIX int forteGlobalDeinitialize();

#ifdef __cplusplus
}
#endif

#endif /* SRC_ARCH_FORTE_C_H_ */
Original file line number Diff line number Diff line change
@@ -1,27 +1,21 @@
/*******************************************************************************
* Copyright (c) 2017 fortiss GmbH
* Copyright (c) 2024 Jose Cabral
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Jose Cabral, Alois Zoitl - initial API and implementation and/or initial documentation
* Jose Cabral - initial API and implementation and/or initial documentation
*******************************************************************************/

#ifndef FORTE_INSTANCE_H_
#define FORTE_INSTANCE_H_
#include "arch/forte_specific_architecture.h"

#ifdef __cplusplus
extern "C" {
#endif

void startupFORTE();
void shutdownFORTE();

#ifdef __cplusplus
int CForteSpecificArchitecture::initialize(int , char** ){
return 0;
}
#endif

#endif
int CForteSpecificArchitecture::deinitialize() {
return 0;
}
15 changes: 11 additions & 4 deletions src/arch/ecos/ecoscppinit.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2010 - 2011 ACIN
* Copyright (c) 2010 - 2024 ACIN, Jose Cabral
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
Expand All @@ -11,11 +11,18 @@
*******************************************************************************/
#include <cyg/kernel/kapi.h>

//current tests have shown that this works only if it is in the main.cpp file
//I don't know why. The best is to copy or include it into your specific main file

//Workaround for an ecos problem
extern "C" void
__cxa_pure_virtual(void) {
//TODO maybe add some exception handling reporting here
}

extern "C" void abort(int ) { while(1) ; }

void *__dso_handle = 0;

extern "C" int _getpid(void) {
return 1;
}

extern "C" void _kill(int ) { while(1) ; }
41 changes: 0 additions & 41 deletions src/arch/ecos/forte_architecture.cpp

This file was deleted.

Loading