undocumented incompatible API change between R7-1 and R7-2

[bfrk]

The sendmsg member of struct driver_table was changed from RTN_STATUS (*sendmsg) (int, char const *, char *); to RTN_STATUS (*sendmsg) (int, char const *, const char *);. Consequently all motor support submodules were changed.

Was that absolutely necessary?

Even if it was, or there were good reasons to make this change, such a API breakage should be documented!

Especially in a case like this, where the error message from the compiler (when building the new motor modules against motor-R7-1) merely points at the struct definition and leaves you guessing which member of the struct is at fault:

/usr/bin/g++  -D_GNU_SOURCE -D_DEFAULT_SOURCE            -D_X86_64_  -DUNIX  -Dlinux     -O3 -g   -Wall      
-mtune=generic     -m64 -fPIC -I. -I../O.Common -I. -I. -I.. -I../../../include/compiler/gcc -I../../../include/os/Linux -I../../../include   
-I/srv/csr/Epics/sumo/build/MOTOR/R7-1+HOBICAT-001/include   
-I/srv/csr/Epics/sumo/build/ASYN/R4-35-bessy2+BII-011/include -I/srv/csr/Epics/sumo/build/BASE/R3-15-6-bessy5+BII-011/include/compiler/gcc 
-I/srv/csr/Epics/sumo/build/BASE/R3-15-6-bessy5+BII-011/include/os/Linux -I/srv/csr/Epics/sumo/build/BASE/R3-15-6-bessy5+BII-011/include        -c ../drvOmsPC68.cc
../drvOmsPC68.cc:148:1: error: invalid conversion from ‘RTN_STATUS (*)(int, const char*, const char*) 
{aka RTN_VALUES (*)(int, const char*, const char*)}’ to ‘RTN_STATUS (*)(int, const char*, char*) {aka RTN_VALUES (*)(int, const char*, char*)}’ [-fpermissive]
 };
 ^
../drvOmsPC68.cc:148:1: error: invalid conversion from ‘const char**’ to ‘char**’ [-fpermissive]

Splitting the motor support into the main motor module and submodules means that the API between them is no longer an internal implementation detail, but a public API. Please treat is as such, otherwise the split makes no sense. In particular:

• Think twice before making a breaking API change
• Communicate such breakage loudly in the release notes and the announcements.

[tboegi]

Yeah, sorry for the extra work caused by this.

The intention was to remove the more than annoying warning/error, that modern compilers give. (And as a side-questeion: Does it help to update the OMS modules from upstream ?)

commit 8b43d40d18679c208381663cc9f51e452a6a40b1 (origin/190920_send_mess_uses_const_char) Author: Torsten Bögershausen torsten.bogershausen@esss.se Date: Mon Sep 30 12:31:09 2019 +0200

send_mess() uses 'const char *' (and more const char*)

The 2nd and 3rd parameter in send_mess() can and should
be a 'const char *' instead of just 'char *'.
Modern compilers complain here, so that the signature now
gets the const.

Update drivers from the following list to use the new send_mess():
    modules/motorAcs
    modules/motorAcsTech80
    modules/motorAerotech
    modules/motorFaulhaber
    modules/motorIms
    modules/motorKohzu
    modules/motorMclennan
    modules/motorMicos
    modules/motorMicroMo
    modules/motorNewFocus
    modules/motorNewport
    modules/motorOms
    modules/motorOriel
    modules/motorPI
    modules/motorParker
    modules/motorPiJena
    modules/motorSmartMotor
    modules/motorThorLabs

[bfrk]

You seem to misunderstand: I used the git head of motorOms against motor-R7-1. There was no indication anywhere that the latest motorOms cannot be built with motor-R7-1. As you can see from the build output I posted, we build the vendor specific motor modules as independent separate support modules. This is supposed to be supported and has worked nicely for us so far.

Again, my point here is to raise awareness among the maintainers that you are dealing with a public API now and that the whole idea of splitting up the motor support only makes sense if one can rely on a stable API.

[kmpeters]

The lack of sufficient documentation for this change in the release notes is my mistake.

The motor release notes are currently the best documentation for which versions of the driver modules are compatible with a version of the motor module. Here are the release notes for motor-R7-1:

https://github.com/epics-modules/motor/releases/tag/R7-1

I will add the latest-supported version of the motor module to the release notes in future releases of the driver modules.

I don't think it is reasonable, however, to expect to master branches of driver modules work with older versions of motor. I do expect the master branch of driver modules to work with the master branch of motor.