Skip to content

Conversation

@rafmudaf
Copy link
Collaborator

@rafmudaf rafmudaf commented Apr 7, 2025

C binding interface utilities and improvements

This pull request initially focused on the development of a module for driving a wave tank experimental setup (see rafmudaf#28). It has since been descoped to focus only on improvements to the C binding interfaces, coupling the WaveField pointer from SeaState to MoorDyn, and initializing the MHK capabilities in AeroDynInflow correctly. I've also extended the new NWTC_C_Binding module (see #2720) with utilities for working with C and Fortran strings plus error handling.

This pull request depends on #2719 and #2720, and both have been merged.

C Bindings

Dealing with strings through the C interfaces is nuances, so I've introduced some tooling and patterns to make this more clear and consistent.

For context, a C-style string is represented in Fortran as CHARACTER(KIND=C_CHAR) :: ErrMsg_c(IntfStrLen). This is a character array of kind C_CHAR from the ISO_C_BINDING intrinsic module with one dimension of length IntfStrLen. This representation is different from the Fortran string, CHARACTER(IntfStrLen) :: ErrMsg, which is a string of length IntfStrLen (not an array). Importantly, C strings in Fortran must be terminated with C_NULL_CHAR from ISO_C_BINDING before being used by a function or subroutine expecting C-style strings: ErrMsg = " "//C_NULL_CHAR.

Since the C binding interfaces operate on both Fortran and C style strings, I've introduced the _C and _F suffix to the error message buffer variables and most other strings crossing the interface for any subroutines that have C bindings. The variable mapping and function is as follows:

Old var New var Description
ErrStat2 ErrStat_F2 Subroutine-local error
ErrStat ErrStat_F Module-global error
ErrStat_C ErrStat_C Error to be passed to the calling code

ErrStat_F2 should hold errors from individual commands, and then it should be incorporated into the module-global error with CALL SetErrStat( ErrStat_F2, ErrMsg_F2, ErrStat_F, ErrMsg_F, RoutineName ). Prior to returning control to the calling code, the _F error variables should be transferred to the _C variables with CALL SetErrStat_F2C(ErrStat_F,ErrMsg_F,ErrStat_C,ErrMsg_C).

An additional error handing utility is added, SetErrStat_C2C, to support glue code modules written in Fortran and combining C binding interfaces such as the WaveTank module. This subroutine expects a C-style error message string for both the local and global error message.

@rafmudaf rafmudaf force-pushed the dev-cbind-wavetank branch from 62ac9d6 to a712957 Compare April 7, 2025 20:33
@andrew-platt andrew-platt self-assigned this Apr 7, 2025
@andrew-platt andrew-platt added this to the v4.1.0 milestone Apr 7, 2025
rafmudaf added 4 commits April 7, 2025 17:38
This allows for doing the geometric offset for MHK turbines; otherwise, the IC are set in Init but the geometric offsets must happen in preinit
@rafmudaf rafmudaf force-pushed the dev-cbind-wavetank branch from 71ad1c8 to efe3c4a Compare April 11, 2025 21:28
@rafmudaf rafmudaf force-pushed the dev-cbind-wavetank branch from efe3c4a to fbff4b2 Compare April 14, 2025 14:54
# Conflicts:
#	reg_tests/r-test
@deslaughter
Copy link
Collaborator

@rafmudaf Could you update the documentation for running the regression tests that use the pyOpenFAST module? It took me a bit to figure out that GitHub actions does pip install glue-codes/python/. and then executePythonRegressionCase.py imports it. Perhaps a better alternative would be to have executePythonRegressionCase.py add glue-codes/python/. to sys.path so it's always using the version directly from the repo. Sorry if this is the wrong place for this feedback, I figured this PR was related to #2720.

@rafmudaf rafmudaf force-pushed the dev-cbind-wavetank branch from 7a21c0b to c98d7a4 Compare May 8, 2025 17:15
Co-authored-by: Andy Platt <andrew-platt@users.noreply.github.com>
@rafmudaf rafmudaf force-pushed the dev-cbind-wavetank branch from dd8388e to 36c02bc Compare May 8, 2025 18:21
@rafmudaf
Copy link
Collaborator Author

@andrew-platt I've updated the NWTC C Bindings module and added tests for each included subroutine. It would be great if you could review it again.

@rafmudaf rafmudaf force-pushed the dev-cbind-wavetank branch 2 times, most recently from 009e13b to 3a65dbf Compare May 13, 2025 02:51
@rafmudaf rafmudaf force-pushed the dev-cbind-wavetank branch from 3a65dbf to 42a180c Compare May 13, 2025 02:52
@rafmudaf rafmudaf force-pushed the dev-cbind-wavetank branch from db270ab to de37045 Compare May 21, 2025 16:23
@andrew-platt andrew-platt changed the base branch from dev-cbind to dev June 16, 2025 18:08
@andrew-platt
Copy link
Collaborator

andrew-platt commented Jun 16, 2025

Postponing this update to 5.0

  • due to external timelines, we need to release 4.1.0 without the API changes to AeroDyn_Inflow_C_Binding.f90 included in this PR.

@andrew-platt andrew-platt modified the milestones: v4.1.0, v5.0.0 Jun 16, 2025
@andrew-platt andrew-platt changed the base branch from dev to dev-cbind June 17, 2025 02:18
@andrew-platt andrew-platt changed the base branch from dev-cbind to dev June 17, 2025 03:15
@andrew-platt andrew-platt requested a review from Copilot July 9, 2025 15:02
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors and enhances the C‐binding interfaces across multiple modules by standardizing error handling, improving C/Fortran string utilities, and adding new capabilities (e.g., exposing the SeaState wave field pointer and MHK support in AeroDynInflow).

  • Introduce SetErrStat_F2C, SetErrStat_C, and robust string conversion routines in NWTC_C_Binding
  • Migrate SeaState, MoorDyn, HydroDyn, InflowWind, and AeroDynInflow C bindings to use the new error and string utilities
  • Extend Python and LabVIEW glue code to match updated interfaces and add MHK/environment parameter support

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
vs-build/_c_binding/.vfproj Add NWTC_C_Binding.f90 to all Fortran‐binding project files
modules/nwtc-library/src/NWTC_C_Binding.f90 Add error‐stat conversion and C/Fortran string helper routines
modules/seastate/src/SeaState_C_Binding.f90 Switch to SetErrStat_F2C, remove old SetErr, export wave field pointer
modules/moordyn/src/MoorDyn_C_Binding.f90 Update error variables, add MD_C_SetWaveFieldData subroutine
modules/aerodyn/src/AeroDyn_Inflow_C_Binding.f90 Switch to new error APIs, remove deprecated SetErr, add MHK flags
glue-codes/python/pyOpenFAST/*.py Update error‐message lengths and function signatures for new C bindings
glue-codes/labview/src/WaveTank.f90 Use SetErrStat_C instead of old SetErr
Comments suppressed due to low confidence (3)

modules/aerodyn/src/AeroDyn_Inflow_C_Binding.f90:59

  • [nitpick] Remove the commented-out SetErr subroutine block; the new SetErrStat_F2C API is in use and the old code is dead and misleading.
   !------------------------------------------------------------------------------------

modules/nwtc-library/tests/test_NWTC_C_Binding.F90:38

  • Extend test_SetErrStat_F2C to also verify that error_message_c contains the correct null-terminated C string (e.g., by checking each character up to the C_NULL_CHAR), ensuring the Fortran-to-C string conversion is validated.
        character(ErrMsgLen) :: error_message_f = "Error message"

modules/seastate/src/SeaState_C_Binding.f90:298

  • This function uses C_LOC and C_PTR but the module does not USE ISO_C_BINDING. Add USE ISO_C_BINDING at the top of the module so that C_LOC and C_PTR are available.
FUNCTION SeaSt_GetWaveFieldPointer_C() BIND (C, NAME='SeaSt_GetWaveFieldPointer_C')

@andrew-platt
Copy link
Collaborator

We can't reproduce the overflow bug in RemoveCStringNullChar (might still be truncating at wrong c_null if multiple). We'll debug that if we hit it again.

@andrew-platt andrew-platt merged commit f072e40 into OpenFAST:dev Jul 11, 2025
12 checks passed
@rafmudaf rafmudaf deleted the dev-cbind-wavetank branch July 11, 2025 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants