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

Ticket3071: Add IOC for Kicker PSU #313

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Conversation

ghost
Copy link

@ghost ghost commented Aug 28, 2018

Description of work

  • Created an IOC for the Kicker PSU.
  • The device has not been configured to talk to the Schneider Electric M580 device to read on/off status but boiler plate code has been added for configuring when we have more information (uses modbus support module).

To test

#3404

Acceptance criteria

  • All RECSIM tests pass. There are no DEVSIM tests.
  • The IOC can read values from the DAQ to be used with the Kicker PSU.
  • The code is of good quality and is robust.

Code Review

Functional Tests

  • IOC responds correctly in:
    • Devsim mode
    • Recsim mode
    • Real device, if available
  • Supplementary IOCs (..._0n where n>1) run correctly
  • Log files do not report undefined macros (serach for macLib: macro to find instances of macLib: macro [macro name] is undefined...

Final steps

  • Update the IOC submodule in the main EPICS repo. See Git workflow page for details.
  • Reviewer has moved the release notes entry for this ticket in the "Changes merged into master" section

Copy link
Contributor

@Alistair-McGann-Tessella Alistair-McGann-Tessella left a comment

Choose a reason for hiding this comment

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

Good separation between the devices makes this very legible. There are a couple of relatively minor tweaks needed, and the modbus settings need to be added.

@@ -0,0 +1,6 @@
## NI cDAQ-9185
epicsEnvSet("CDAQAI","cDAQ9185-1D195CFMod3")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a hard-coded address for this specific hardware unit, or is this a more general model number which would allow this IOC to be used by other instruments or facilities? If it is tied to a single unit, could this be made more generic through macros?

epicsEnvSet("CDAQAI","cDAQ9185-1D195CFMod3")

## input
$(IFNOTRECSIM) DAQmxConfig("R0", "$(CDAQAI)/ai0", 0, "AI","N=1000 F=1000") ## Kicker Volt
Copy link
Contributor

Choose a reason for hiding this comment

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

The N and F values will be important when analysing data from the DAQ, and I think are probably of use to the instrument scientists as well. Would it be possible to set these as macros? It may even be worth writing these values to a PV.

< $(IOCSTARTUP)/dbload.cmd

# Load up DB records for talking to the DAQ box
< iocBoot/iocKICKER-IOC-01/st-daq.cmd
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 I like that the two devices are separated out into different files

< iocBoot/iocKICKER-IOC-01/st-schneider.cmd

## Load our record instances
dbLoadRecords("$(KICKER)/db/kicker.db","PVPREFIX=$(MYPVPREFIX), ,P=$(MYPVPREFIX)$(IOCNAME):,DAQMX=$(MYPVPREFIX)$(IOCNAME):DAQ:, PSU_MAX_CURR=$(PSU_MAX_CURR=15),RECSIM=$(RECSIM=0),DISABLE=$(DISABLE=0),PORT=$(DEVICE)")
Copy link
Contributor

Choose a reason for hiding this comment

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

PV names in IBEX tend to start with $(P), which is superceeded quite often here with $(DAQMX). If there is a need to do this, could this be placed in a comment somewhere in the code?

$(IFRECSIM) drvAsynSerialPortConfigure("$(DEVICE):PLC", "$(PORT=NUL)", 0, 1, 0, 0)

## For real devices
$(IFNOTRECSIM) drvAsynSerialPortConfigure("$(DEVICE):PLC","$(PORT=NO_PORT_MACRO)",0,0,0) # change this
Copy link
Contributor

Choose a reason for hiding this comment

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

I had a poke around the schneider M580 documentation here, which suggests that it communicates Modbus over ethernet. It's not a big change if it is, just something to look out for when you hook everything up

< $(IOCSTARTUP)/init.cmd

## Modbus configuration
drvModbusAsynConfigure("$(DEVICE)heartbeat", "$(DEVICE)", 1, 3, 649, 1, 0, 1000, "PLC")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this modbus port definition be moved to the st-schneider.cmd file with the other one? Perhaps with a comment explaining what they both intend to access

KICKER/iocBoot/iocKICKER-IOC-01/st.cmd Show resolved Hide resolved

## Load our record instances
dbLoadRecords("$(KICKER)/db/kicker.db","PVPREFIX=$(MYPVPREFIX), ,P=$(MYPVPREFIX)$(IOCNAME):,DAQMX=$(MYPVPREFIX)$(IOCNAME):DAQ:, PSU_MAX_CURR=$(PSU_MAX_CURR=15),RECSIM=$(RECSIM=0),DISABLE=$(DISABLE=0),PORT=$(DEVICE)")
dbLoadRecords("$(KICKER)/db/kicker_voltage.db","PVPREFIX=$(MYPVPREFIX),P=$(MYPVPREFIX)$(IOCNAME):,RECSIM=$(RECSIM=0),DISABLE=$(DISABLE=0),PORT=$(DEVICE),DAQMX=$(MYPVPREFIX)$(IOCNAME):DAQ:,IFNOTRECSIM=$(IFNOTRECSIM),IFRECSIM=$(IFRECSIM),PSU_MAX_VOLT=$(PSU_MAX_VOLT=45),NELM=$(NELM=1000)")
Copy link
Contributor

Choose a reason for hiding this comment

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

Related to the comment about letting the user set F and N in the st-daq.cmd, there is already a macro here which appears to set N, NELM. I think this may get confusing though, as the end users might not understand what this means/refers to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant