Edited OERCONE ioc to boot using lua, calling scripts from the utilities support modules #433

JamesKingWork · 2019-10-02T17:49:33Z

The OERCONE ioc now boots partially by using lua which is called from the st-common.cmd

Some of the code is messy as I have not had time for cleanup yet, but it is tested and works against the IocTestFramework.

There is a strong code smell in here (dead code) it is up to the reviewer whether they think it important I should get rid of this smell.
The 3 reasons I have not removed the dead code are:

For a more complete switch to lua we could call st.lua from the cpp that boots the ioc. The code left in the cpp file and st.lua could be useful for that at a later date (though of course this can all be removed and cleaned up if the reviewer thinks it correct)
I have left the original functionality in the st-common.cmd as it both shows the mapping of cmd code to lua code and also if the lua functionality when tested against hardware for whatever reason does not work we can easily switch back to cmd.
I haven't had time on this timeboxed friday and could pick this ticket up to make it more elegant (removing dead code) in the next friday, so it may make more sense to leave the extra code in there so I can make easy progress for next time

Description of work

Edited the IOC for OERCONE to boot with lua called in the st-common.cmd

To test

Use the IOCTestFramework to test the oercone and eventually against hardware hopefully.

Ticket: (ISISComputingGroup/IBEX#4288)

Acceptance criteria

IOC st.cmd uses LUA instead
Database is still updated
Devsim and rec sim tests still run in the IOC test framework
Log is still captured as log file and in the database
Errors in missing macros, db files are still reported

Code Review

A copy of the manual has been placed on the shared drive
Pertitent information has been stored in the wiki
Does the IOC conform to IBEX standards?
If an OPI has been modified, does it conform to the style guidelines?
Do the IOC and support module conform to the new build guidelines
Have the changes been recorded appropriately in the release notes?
Is the device's flow control setting correct? For most devices flow control should be OFF.

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

…ies support modules

DominicOram

This is good work. As discussed can you remove the unfinished C code and remove the unused *.cmd code. Other minor comments in code.

DominicOram · 2019-10-03T10:03:35Z

OERCONE/OERCONE-IOC-01App/src/OERCONE-IOC-01Main.cpp


 #include "epicsExit.h"
 #include "epicsThread.h"
 #include "iocsh.h"
+#include "luaShell.h"


As discussed remove this C code.

DominicOram · 2019-10-03T10:03:56Z

OERCONE/iocBoot/iocOERCONE-IOC-01/st-common.cmd

@@ -1,46 +1,50 @@
-epicsEnvSet "STREAM_PROTOCOL_PATH" "$(OERCONE)/data"


As discussed, remove unused code in this file

DominicOram · 2019-10-03T10:31:13Z

OERCONE/iocBoot/iocOERCONE-IOC-01/st-common.lua

+iocsh.epicsEnvSet("STREAM_PROTOCOL_PATH", string.format("%s/data", oercone))
+iocsh.epicsEnvSet("DEVICE", "L0")


I think these variables are not used anywhere other than this file so need to set them externally.

Changed to local variables and removed stream protocol path as it is not used

It turns out that STREAM_PROTOCOL_PATH and DEVICE epics environment variables are used elsewhere, the tests fail otherwise.

DominicOram · 2019-10-03T10:32:39Z

OERCONE/iocBoot/iocOERCONE-IOC-01/st-common.lua

+devsim = getMacroValue{macro="DEVSIM", default="0"}
+isDevsim = devsim == "1"


We actually only need isDevsim so can we just do this in one line? Similar for recsim

Changed to be one line for isDevsim.
recsim variable itself is used in iocsh.dbLoadRecords though, so kept that.

DominicOram · 2019-10-03T14:43:20Z

OERCONE/iocBoot/iocOERCONE-IOC-01/st-common.lua

+-- ##ISIS## Stuff that needs to be done after all records are loaded but before iocInit is called 
+iocsh.iocshLoad(string.format("%s/preiocinit.cmd", iocstartup))
+
+-- os.execute(string.format("cd %s/iocBoot/%s", getMacroValue{macro="TOP"}, getMacroValue{macro="IOC"}))


Commented code, remove

DominicOram · 2019-10-03T14:43:29Z

OERCONE/iocBoot/iocOERCONE-IOC-01/st-common.lua

+-- ## Start any sequence programs
+-- #seq sncxxx,"user=ltu34219"


Commented code, remove

DominicOram · 2019-10-03T14:44:02Z

OERCONE/iocBoot/iocOERCONE-IOC-01/st.lua

+
+-- # calling common command file in ioc 01 boot dir
+-- # < ${TOP}/iocBoot/iocOERCONE-IOC-01/st-common.cmd
+require("st-common.lua")


Need a new line

The st.lua file is never called so was dead code.
Removed the file.

DominicOram · 2019-10-03T14:53:59Z

OERCONE/iocBoot/iocOERCONE-IOC-01/st.lua

+-- # Increase this if you get <<TRUNCATED>> or discarded messages warnings in your errlog output
+iocsh.errlogInit2(65536, 256)
+
+-- # envPaths


Remove commented out code

The st.lua file is never called so was dead code.
Removed the file.

DominicOram · 2019-10-03T14:54:07Z

OERCONE/iocBoot/iocOERCONE-IOC-01/st.lua

+-- # envPaths
+iocsh.iocshLoad(string.format("%s/dbload.cmd", iocstartup))
+
+-- # cd ${TOP}


Remove commented out code

The st.lua file is never called so was dead code.
Removed the file.

JamesKingWork · 2019-10-17T08:50:16Z

There have been some changes since the review due to unforeseen strangeness in the way epics runs Lua. This included moving the script into a function so that local variables don't drop out of scope. Please see documented issues at the bottom of the Lua wiki page. There is also now documentation linked to here for installing and setting up luacheck, and also documentation for how to use our Lua utility functions.

… scope of the variables in the script

Requested changes have been implemented

Edited OERCONE ioc to boot using lua, calling scripts from the utilit…

f2ec61a

…ies support modules

pull-request-size bot added the size/L label Oct 2, 2019

JamesKingWork mentioned this pull request Oct 2, 2019

IOC: Convert an IOC to Lua ISISComputingGroup/IBEX#4288

Closed

DominicOram previously requested changes Oct 3, 2019

View reviewed changes

DominicOram reviewed Oct 3, 2019

View reviewed changes

JamesKingWork added 4 commits October 14, 2019 09:28

Removed unsused c code

e6191d2

Removed unused cmd code

d3c9138

Removed unnecessary files, tidying up st.cmd and st-common.lua

3c477ca

Conformed to luacheck guidelines using local, changed use of port macro

cbbb20d

Changed the st-common boot script to be in a function to restrict the…

12f9377

… scope of the variables in the script

Alistair-McGann-Tessella approved these changes Nov 5, 2019

View reviewed changes

Alistair-McGann-Tessella merged commit 486dab4 into master Nov 5, 2019

Alistair-McGann-Tessella deleted the Ticket4288_oercone_ioc_to_lua branch November 5, 2019 13:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Edited OERCONE ioc to boot using lua, calling scripts from the utilities support modules #433

Edited OERCONE ioc to boot using lua, calling scripts from the utilities support modules #433

JamesKingWork commented Oct 2, 2019 •

edited

Loading

DominicOram left a comment

DominicOram Oct 3, 2019

JamesKingWork Oct 15, 2019

DominicOram Oct 3, 2019

JamesKingWork Oct 15, 2019

DominicOram Oct 3, 2019

JamesKingWork Oct 15, 2019

JamesKingWork Oct 17, 2019

DominicOram Oct 3, 2019

JamesKingWork Oct 15, 2019

DominicOram Oct 3, 2019

JamesKingWork Oct 15, 2019

DominicOram Oct 3, 2019

JamesKingWork Oct 15, 2019

DominicOram Oct 3, 2019

JamesKingWork Oct 15, 2019

DominicOram Oct 3, 2019

JamesKingWork Oct 15, 2019

DominicOram Oct 3, 2019

JamesKingWork Oct 15, 2019

JamesKingWork commented Oct 17, 2019 •

edited

Loading

		@@ -1,46 +1,50 @@
		epicsEnvSet "STREAM_PROTOCOL_PATH" "$(OERCONE)/data"

		iocsh.epicsEnvSet("STREAM_PROTOCOL_PATH", string.format("%s/data", oercone))
		iocsh.epicsEnvSet("DEVICE", "L0")

		devsim = getMacroValue{macro="DEVSIM", default="0"}
		isDevsim = devsim == "1"

		-- ## Start any sequence programs
		-- #seq sncxxx,"user=ltu34219"

Edited OERCONE ioc to boot using lua, calling scripts from the utilities support modules #433

Edited OERCONE ioc to boot using lua, calling scripts from the utilities support modules #433

Conversation

JamesKingWork commented Oct 2, 2019 • edited Loading

Description of work

To test

Acceptance criteria

Code Review

Functional Tests

Final steps

DominicOram left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JamesKingWork commented Oct 17, 2019 • edited Loading

JamesKingWork commented Oct 2, 2019 •

edited

Loading

JamesKingWork commented Oct 17, 2019 •

edited

Loading