epics-motor / motorNPoint

EPICS motor drivers for nPoint controllers
0 stars 4 forks source link

Add LC400 motor support #1

Closed domitto closed 4 years ago

domitto commented 5 years ago

This PR will add support for the nPoint LC400 piezo motor controller. It does point-to-point movement at a user-defined velocity by uploading a custom waveform with the generated trajectory. It can use either asynPortDriver or asynFTDIPort (https://github.com/epics-modules/asyn/pull/88), the second one is preferred for better transfer speeds.

kmpeters commented 5 years ago

Thanks for this pull request.

domitto commented 4 years ago

Hi Kevin, Don't merge yet, there's a possible firmware issue that the manufacturer is trying to replicate. I have a work-around that I can push but I'd like it to be fixed in the motor controller firmware. I'll keep you posted.

Thanks, Diego

kmpeters commented 4 years ago

Sounds good. I'm happy to wait for the issue to be resolved the right way.

Sent from my Android device with K-9 Mail. Please excuse my brevity.

kmpeters commented 4 years ago

@domitto, has the manufacturer resolved the issue in firmware?

domitto commented 4 years ago

Hi @kmpeters, yes, it was solved a long time ago and it slipped under my radar. I'll fix the PR soon, thanks for reminding me!

domitto commented 4 years ago

@kmpeters, it should be good to go now.

kmpeters commented 4 years ago

@domitto, I have a comment and a couple questions:

DRV_FTDI=YES in CONFIG_SITE should be commented out by default, so the build doesn't fail if the ftdi library is missing.

It looks like the FTDI support is generic (there is no LC400-specific code in it). Is this correct? If so, it might be good to add the FTDI support to asyn, instead of motor, so that it can be used by other drivers...

MarkRivers commented 4 years ago

It looks like the FTDI support is generic (there is no LC400-specific code in it). Is this correct? If so, it might be good to add the FTDI support to asyn, instead of motor, so that it can be used by other drivers...

There is already FTDI support in asyn. It looks like the same files as here. Why?

Also, this commit has lots of OPI files that are duplicates of those in asyn, e.g.

opi/asyn/asynGPIBSetup.opi

Why are those here?

domitto commented 4 years ago

Sorry, I was working on it before the asynFTDI was added. I'll clean up and remove the duplicated OPIs too.

domitto commented 4 years ago

@kmpeters please take a look again, I think I removed all the FTDI sources and cleaned the screens that Mark mentioned.

domitto commented 4 years ago

Hi @MarkRiver, I've moved the opis to the location you suggested.

Thanks, Diego

domitto commented 4 years ago

@kmpeters, I'm not so sure about what to do with this packed error on the windows build as I don't have a windows machine or the hardware to test the changes anymore.

MarkRivers commented 4 years ago

@domitto you can fix that problem by using

#pragma pack(1)

rather than

__attribute__((__packed__));

See this code in the mca module for an example. https://github.com/epics-modules/mca/blob/eb635637c5c295340a2e319ae97ba08658ea9595/mcaApp/CanberraSrc/ncp_comm_defs.h#L40

After the packed structure definitions you revert to previous packing with

#pragma pack()

https://github.com/epics-modules/mca/blob/eb635637c5c295340a2e319ae97ba08658ea9595/mcaApp/CanberraSrc/ncp_comm_defs.h#L269

domitto commented 4 years ago

Thank you @MarkRivers! All checks have passed now.

kmpeters commented 4 years ago

@domitto, the auxParameters.db file is not currently in a place where it can be easily included in other IOCs nor is it obvious that the file shouldn't be used with the C300. It should be moved from here:

motorNPoint/iocs/nPointIOC/iocBoot/iocNPoint/auxParameters.db

To the database directory:

motorNPoint/nPointApp/Db/LC400_auxParameters.db

motorNPoint/nPointApp/Db/Makefile should be modified to install the database file:

DB += LC400_auxParameters.db

motorNPoint/iocs/nPointIOC/nPointApp/Db/Makefile should be modified to install the database based on whether or not MOTOR_NPOINT is defined. An example of how to do this is here:

https://github.com/epics-motor/motorNewport/blob/master/iocs/newportIOC/newportApp/Db/Makefile#L25-L53

And the dbLoadRecords lines in motorNPoint/iocs/nPointIOC/iocBoot/iocNPoint/LC400.cmd should change to this:

dbLoadRecords("$(TOP)/db/LC400_auxParameters.db","PORT=LC400,ADDR=0,P=nPoint:,R=LC400:")
dbLoadRecords("$(TOP)/db/LC400_auxParameters.db","PORT=LC400,ADDR=1,P=nPoint:,R=LC400:")
dbLoadRecords("$(TOP)/db/LC400_auxParameters.db","PORT=LC400,ADDR=2,P=nPoint:,R=LC400:")

This will allow the LC400_auxParameters.db file to be found in the motor/db directory when motorNPoint is built as a submodule of motor and in the motorNPoint/db directory when motorNPoint is build outside of the motor tree.

domitto commented 4 years ago

@kmpeters, I've changed the db and makefile according to your suggestions.

kmpeters commented 4 years ago

@domitto, thanks again for this pull request.