diff --git a/.ci-local/asyn-tirpc.sh b/.ci-local/asyn-tirpc.sh new file mode 100755 index 0000000..b38738e --- /dev/null +++ b/.ci-local/asyn-tirpc.sh @@ -0,0 +1,3 @@ +#!/usr/bin/env bash + +echo "TIRPC=YES" > configure/CONFIG_SITE.Common.linux-x86_64 diff --git a/.ci-local/deps.set b/.ci-local/deps.set new file mode 100644 index 0000000..d253804 --- /dev/null +++ b/.ci-local/deps.set @@ -0,0 +1,10 @@ +MODULES="asyn modbus" + +ASYN=master +MODBUS=master + +# FIXME: Motor module build fails in mingw/windows. Only needed for motor support though +#MOTOR=master + +# Enable libtirpc for Linux builds +ASYN_HOOK=.ci-local/asyn-tirpc.sh diff --git a/.clang-tidy b/.clang-tidy index 4287a83..0f1c9db 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,3 +1,32 @@ --- -Checks: 'clang-diagnostic-*,clang-analyzer-*,cert-*,performance-*' +# See: https://clang.llvm.org/extra/clang-tidy/checks/list.html +Checks: + - 'clang-diagnostic-*' + - 'clang-analyzer-*' + # All CERT checks that clang-tidy supports are relevant + - 'cert-*' + - 'performance-*' + # Validate return value checks for POSIX functions + - 'bugprone-posix-return' + - 'bugprone-sizeof-*' + - 'bugprone-too-small-loop-variable' + - 'bugprone-swapped-arguments' + # Common mistakes due to reversed parameters, bad sizeof's and such + - 'bugprone-suspicious-*' + # Avoid RAII misuse (i.e. forgetting to name mutex guard) + - 'bugprone-unused-raii' + # Flag possible ODR violations + - 'misc-definitions-in-headers' + - 'bugprone-use-after-move' + - 'bugprone-unused-local-non-trivial-variable' + - 'bugprone-not-null-terminated-result' + - 'bugprone-branch-clone' + # Flag potentially bad copy constructors + - 'bugprone-copy-constructor-init' + - 'bugprone-dangling-handle' + - 'bugprone-chained-comparison' + # Avoid cyclic includes + - 'misc-header-include-cycle' + # Get rid of deprecated smart pointer types + - 'modernize-replace-auto-ptr' WarningsAsErrors: '*' diff --git a/.clang-tidy-exclude b/.clang-tidy-exclude new file mode 100644 index 0000000..6788be8 --- /dev/null +++ b/.clang-tidy-exclude @@ -0,0 +1,3 @@ +# Defines patterns to exclude from the clang-tidy run +Exclude: + - '\S+registerRecordDeviceDriver.cpp' diff --git a/.clang-tidy-exclude.json b/.clang-tidy-exclude.json deleted file mode 100644 index acada2e..0000000 --- a/.clang-tidy-exclude.json +++ /dev/null @@ -1,3 +0,0 @@ -[ - "\\S+registerRecordDeviceDriver.cpp" -] diff --git a/.editorconfig b/.editorconfig index ab82c36..cb12e47 100644 --- a/.editorconfig +++ b/.editorconfig @@ -13,3 +13,7 @@ indent_style = space [*.yml] indent_size = 2 indent_style = space + +[.clang-*] +indent_size = 2 +indent_style = space diff --git a/.github/workflows/ci-scripts-build.yml b/.github/workflows/ci-scripts-build.yml index 2bb9da2..abbdb58 100644 --- a/.github/workflows/ci-scripts-build.yml +++ b/.github/workflows/ci-scripts-build.yml @@ -7,11 +7,11 @@ name: EK9000 -# Trigger on pushes and PRs to any branch +# Trigger on pushes and PRs to any branch, but ignore docs on: push: paths-ignore: - - 'documentation/*' + - 'docs/*' - '**/*.html' - '**/*.md' pull_request: @@ -22,18 +22,15 @@ env: APT: re2c CHOCO: re2c BREW: re2c - MODULES: "asyn modbus" - ASYN: master - MODBUS: master - # FIXME: Motor module build fails in mingw/windows. Only needed for motor support though - #MOTOR: master + # This is where we define our dependencies in CI + SET: deps jobs: format: name: clang-format formatting check runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: clang-format check uses: jidicula/clang-format-action@v4.8.0 with: @@ -42,10 +39,12 @@ jobs: exclude-regex: 'terminals.h' scripts-check: + # Run basic checks on the Python scripts bundled here. + # Checks the terminals.json against the schema as well. name: Scripts Check runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: actions/setup-python@v4 with: python-version: '3.10' @@ -71,51 +70,54 @@ jobs: fail-fast: false matrix: # Job names also name artifacts, character limitations apply + # Ubuntu 22.04: + # clang, clang-13, clang-14, clang-15 (clang=clang-14) + # gcc, gcc-9, gcc-10, gcc-12, gcc-13 (gcc=gcc-11) include: - - os: ubuntu-20.04 + - os: ubuntu-22.04 cmp: gcc configuration: default wine: "64" - name: "Ub-20 gcc-9 + MinGW" + name: "Ub-22 gcc-11 + MinGW" - - os: ubuntu-20.04 + - os: ubuntu-22.04 cmp: gcc configuration: static extra: "CMD_CXXFLAGS=-std=c++14" name: "clang-tidy checks" clang-tidy: true - - os: ubuntu-20.04 + - os: ubuntu-22.04 cmp: gcc configuration: static extra: "CMD_CXXFLAGS=-std=c++14" - name: "Ub-20 gcc-9 C++14, static" + name: "Ub-22 gcc-11 C++14, static" - - os: ubuntu-20.04 + - os: ubuntu-22.04 cmp: gcc configuration: static extra: "CMD_CXXFLAGS=-std=c++03" - name: "Ub-20 gcc-9 C++03, static" + name: "Ub-22 gcc-11 C++03, static" - - os: ubuntu-20.04 + - os: ubuntu-22.04 cmp: clang configuration: default extra: "CMD_CXXFLAGS=-std=c++14" - name: "Ub-20 clang-10 C++14" + name: "Ub-22 clang-14 C++14" - - os: ubuntu-20.04 + - os: ubuntu-22.04 cmp: gcc configuration: default rtems: "5" rtems_target: RTEMS-beatnik test: NO - name: "Ub-20 gcc-9 + RT-5.1 beatnik" + name: "Ub-22 gcc-11 + RT-5.1 beatnik" - - os: ubuntu-20.04 + - os: ubuntu-22.04 cmp: clang configuration: default extra: "CMD_CXXFLAGS=-std=c++03" - name: "Ub-20 clang-10 C++03" + name: "Ub-22 clang-14 C++03" - os: macos-latest cmp: clang @@ -128,16 +130,21 @@ jobs: name: "Win2019 MinGW" steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 with: submodules: true + - name: Automatic core dumper analysis uses: mdavidsaver/ci-core-dumper@master + + # Install dependencies. Qemu for RTEMS, mingw GCC for Windows cross builds and bear to generate compile_commands.json for clang-tidy - name: "apt-get install" run: | sudo apt-get update sudo apt-get -y install qemu-system-x86 g++-mingw-w64-x86-64 gdb bear clang-tidy if: runner.os == 'Linux' + + # Install required cross toolchain for RTEMS - name: "apt-get install ${{ matrix.cmp }}" run: | sudo apt-get -y install software-properties-common @@ -145,22 +152,32 @@ jobs: sudo apt-get update sudo apt-get -y install ${{ matrix.cmp }} if: matrix.utoolchain + - name: Prepare and compile dependencies run: python .ci/cue.py prepare + - name: Build main module run: python .ci/cue.py build + + # Run clang-tidy on our code. To do this, we need compile_commands.json, which we generate using 'bear' + # To get a complete compile_commands.json, we need to re-run the entire compile with bear. For now, we'll only do this on Linux native builds. - name: Run clang-tidy run: | + python3 -m pip install -r scripts/requirements.txt make clean - bear make -j$(nproc) - python3 clang-tidy.py + bear -- make -j$(nproc) + python3 scripts/clang-tidy.py if: matrix.clang-tidy + - name: Run main module tests run: python .ci/cue.py test + + # Resulting test files will be uploaded and attached to this run - name: Upload tapfiles Artifact uses: actions/upload-artifact@v2 with: name: tapfiles ${{ matrix.name }} path: '**/O.*/*.tap' + - name: Collect and show test results run: python .ci/cue.py test-results diff --git a/clang-tidy.py b/clang-tidy.py deleted file mode 100644 index e3b6b8b..0000000 --- a/clang-tidy.py +++ /dev/null @@ -1,32 +0,0 @@ -#!/usr/env/bin python3 - -import json -import subprocess -import re - -if __name__ == '__main__': - exclude = [] - with open('.clang-tidy-exclude.json', 'rb') as fp: - exclude = json.load(fp) - - assert isinstance(exclude, list) - - def is_excluded(file: str) -> bool: - for reg in exclude: - if re.search(reg, file) is not None: - return True - return False - - returnCode = 0 - with open('compile_commands.json', 'rb') as fp: - f = json.load(fp) - for cmd in f: - file = cmd['file'] - if not is_excluded(file): - print(f'Running clang-tidy on {file}') - r = subprocess.run(['clang-tidy', file], cwd=cmd['directory']) - if r.returncode != 0: - returnCode = r.returncode - else: - print(f'Skipped {file}') - exit(returnCode) diff --git a/configure/RELEASE b/configure/RELEASE index db2e372..bab7807 100644 --- a/configure/RELEASE +++ b/configure/RELEASE @@ -41,8 +41,8 @@ # Don't set your version to anything such as "test" that # could match a directory name. # ========================================================== -ASYN_MODULE_VERSION=R4.39-1.0.1 -MODBUS_MODULE_VERSION=R3.2-1.0.1 +ASYN_MODULE_VERSION=R4.39-1.0.2 +MODBUS_MODULE_VERSION=R3.2-1.0.2 # ========================================================== # Define module paths using pattern diff --git a/ek9000App/src/devEL3XXX.cpp b/ek9000App/src/devEL3XXX.cpp index 0b3cead..72d5be3 100644 --- a/ek9000App/src/devEL3XXX.cpp +++ b/ek9000App/src/devEL3XXX.cpp @@ -131,7 +131,8 @@ struct EL30XXStandardInputPDO_t { uint8_t _r2 : 6; // Last bit in this align is Sync error for EL31XX uint8_t txpdo_state : 1; uint8_t txpdo_toggle : 1; - uint16_t value; + int16_t value; // Must be signed to accommodate bipolar terminals. Unsigned representation still defines range as + // 0-32767, so this is safe. }; #pragma pack() @@ -339,7 +340,7 @@ struct EL331XInputPDO_t { uint8_t error : 1; uint8_t txpdo_state : 1; uint8_t txpdo_toggle : 1; - uint16_t value; + int16_t value; }; struct EL3314_0010_InputPDO_t { @@ -351,7 +352,7 @@ struct EL3314_0010_InputPDO_t { uint8_t txpdo_state : 1; uint8_t txpdo_toggle : 1; uint8_t padding1 : 7; // Pad it out to byte boundary - uint16_t value; + int16_t value; }; #pragma pack() diff --git a/ek9000App/src/devEL4XXX.cpp b/ek9000App/src/devEL4XXX.cpp index 7082150..b52a5ef 100644 --- a/ek9000App/src/devEL4XXX.cpp +++ b/ek9000App/src/devEL4XXX.cpp @@ -39,6 +39,10 @@ static long EL40XX_init_record(void* record); static long EL40XX_write_record(void* record); static long EL40XX_linconv(void* precord, int after); +struct EL40XXDpvt_t : public TerminalDpvt_t { + bool sign = false; +}; + struct devEL40XX_t { long num; DEVSUPFUN report; @@ -54,6 +58,11 @@ struct devEL40XX_t { epicsExportAddress(dset, devEL40XX); +// The default representation for all of these terminals is signed. Unsigned may also be set, even for the bipolar +// terminals that may produce a negative value. To retain some level of support for unsigned representation, terminals +// that have a positive output range use uint16_t as the PDO type. Bipolar terminals always use int16_t to support +// negative values and will behave incorrectly if you choose the unsigned (or absolute w/MSB sign) representation. + DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4001); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4002); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4004); @@ -66,23 +75,33 @@ DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4021); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4022); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4024); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4028); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4031); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4032); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4034); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4038); +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4031); // EL403X support negative output values. +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4032); +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4034); +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4038); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4102); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4104); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4112); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4114); +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4112); // EL411X support negative output values. +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4114); DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4122); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4132); -DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(uint16_t, EL4134); +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4132); // EL413X support negative output values. +DEFINE_SINGLE_CHANNEL_OUTPUT_PDO(int16_t, EL4134); + +static bool isTerminalSigned(int id) { + if (id <= 4039 && id >= 4030) + return true; // EL403X + if (id <= 4119 && id >= 4110) + return true; // EL411X + if (id <= 4139 && id >= 4130) + return true; // EL413X + return false; +} static void EL40XX_WriteCallback(CALLBACK* callback) { void* record = NULL; callbackGetUser(record, callback); aoRecord* pRecord = (aoRecord*)record; - TerminalDpvt_t* dpvt = (TerminalDpvt_t*)pRecord->dpvt; + EL40XXDpvt_t* dpvt = (EL40XXDpvt_t*)pRecord->dpvt; free(callback); int status = 0; @@ -103,9 +122,14 @@ static void EL40XX_WriteCallback(CALLBACK* callback) { } /* Set buffer & do write */ - uint16_t buf = (int16_t)pRecord->rval; + char buf[2]; + if (dpvt->sign) + *(int16_t*)buf = (int16_t)pRecord->rval; + else + *(uint16_t*)buf = (uint16_t)pRecord->rval; + status = dpvt->pterm->doEK9000IO(MODBUS_WRITE_MULTIPLE_REGISTERS, - dpvt->pterm->m_outputStart + (dpvt->channel - 1), &buf, 1); + dpvt->pterm->m_outputStart + (dpvt->channel - 1), (uint16_t*)buf, 1); } /* Check error */ @@ -135,7 +159,7 @@ static long EL40XX_init(int) { static long EL40XX_init_record(void* record) { aoRecord* pRecord = (aoRecord*)record; pRecord->dpvt = util::allocDpvt(); - TerminalDpvt_t* dpvt = (TerminalDpvt_t*)pRecord->dpvt; + EL40XXDpvt_t* dpvt = (EL40XXDpvt_t*)pRecord->dpvt; uint16_t termid = 0; /* Verify terminal */ @@ -164,6 +188,9 @@ static long EL40XX_init_record(void* record) { return 1; } + /* Determine if it's signed or not */ + dpvt->sign = isTerminalSigned(termid); + return 0; } diff --git a/clang-format.sh b/scripts/clang-format.sh similarity index 100% rename from clang-format.sh rename to scripts/clang-format.sh diff --git a/scripts/clang-tidy.py b/scripts/clang-tidy.py new file mode 100755 index 0000000..d59ccdc --- /dev/null +++ b/scripts/clang-tidy.py @@ -0,0 +1,56 @@ +#!/usr/env/bin python3 + +import json +import subprocess +import re +import argparse +import yaml + +def _load_config() -> dict: + """ + Loads the config off of disk. This is required especially for older versions of clang-tidy that do not support the new format, which + allows for a nice list of checks instead of a big long comma delimited string + """ + with open('.clang-tidy', 'r') as fp: + f = yaml.safe_load(fp) + f['Checks'] = ','.join(f['Checks']) # Convert from a list to a comma delimited str + return f + + +if __name__ == '__main__': + parser = argparse.ArgumentParser() + parser.add_argument('--clang-tidy', type=str, dest='clang_tidy', default='clang-tidy', help='Clang tidy executable to use') + args = parser.parse_args() + + r = subprocess.run([args.clang_tidy, '--version'], capture_output=True) + if r.returncode != 0: + print(f'{args.clang_tidy} not found') + exit(1) + + exclude = [] + with open('.clang-tidy-exclude', 'rb') as fp: + exclude = yaml.safe_load(fp)['Exclude'] + + assert isinstance(exclude, list) + + def is_excluded(file: str) -> bool: + for reg in exclude: + if re.search(reg, file) is not None: + return True + return False + + conf = json.dumps(_load_config()) + + returnCode = 0 + with open('compile_commands.json', 'rb') as fp: + f = json.load(fp) + for cmd in f: + file = cmd['file'] + if not is_excluded(file): + print(f'Running clang-tidy on {file}') + r = subprocess.run([args.clang_tidy, f'--config={conf}', file], cwd=cmd['directory']) + if r.returncode != 0: + returnCode = r.returncode + else: + print(f'Skipped {file}') + exit(returnCode) diff --git a/scripts/requirements.txt b/scripts/requirements.txt new file mode 100644 index 0000000..c3726e8 --- /dev/null +++ b/scripts/requirements.txt @@ -0,0 +1 @@ +pyyaml