Opcua IOC #57
Opcua IOC #57
Conversation
guirodrigueslima
commented
Apr 10, 2024
guirodrigueslima
commented
- Added opca ioc to epics-in-docker
- The IOC was not build with the install_module() function, as "make clean" removes the envPaths file.
|
Added ether_ip IOC to PLCs Rockwell family. |
guirodrigueslima
force-pushed
the
opcua
branch
2 times, most recently
from
690e5e2 to
7de7014
Compare
There was a problem hiding this comment.
Besides the implementation, I think we can improve the messages by making them more descriptive and explanatory. For instance, why we are using another path for IOCs? Why do we now use another function to install things?
In addition, please let the first letter after the : lowercase and ending with a period (.), so that the message headline is kept in the pattern.
Sorry. I think I wasn't clear enough here. I'm talking about commit messages specifically. (Same thing for the the lowercase and period thing I said.) |
guirodrigueslima
force-pushed
the
opcua
branch
2 times, most recently
from
db8b9d9 to
a0b7476
Compare
base/Dockerfile
Outdated
| @@ -27,6 +28,7 @@ COPY lnls-run.sh /usr/local/bin/lnls-run | |||
| ARG EPICS_BASE_VERSION | |||
| ENV EPICS_BASE_PATH /opt/epics/base | |||
| ENV EPICS_MODULES_PATH /opt/epics/modules | |||
| ENV EPICS_IOCS_PATH /opt/epics/iocs | |||
There was a problem hiding this comment.
We no longer need this definition, right?
CHANGES.md
Outdated
| * base: new functionality for installing iocs. by @guirodrigueslima in | ||
| https://github.com/cnpem/epics-in-docker/pull/57 |
There was a problem hiding this comment.
Again, you don't need to add this to the changelog. Only the one from commit a0b7476.
base/docker-compose.yml
Outdated
| @@ -29,3 +29,4 @@ services: | |||
| PMAC_VERSION: ${PMAC_VERSION} | |||
| LIBSSCPIMEGA_VERSION: ${LIBSSCPIMEGA_VERSION} | |||
| RETOOLS_VERSION: ${RETOOLS_VERSION} | |||
|
|
|||
There was a problem hiding this comment.
Drop this empty line change from this commit.
There was a problem hiding this comment.
Everything looks fine by me. @ericonr, can you finish your review on this for us to merge it?
Highlighted some points that can be fixed when merging. No need for another patch iteration for these.
The modules should be grouped by script which uses them.
This way, the arguments are grouped by which script uses them, and can explicitly follow the same order of definition used in the .env and docker-compose.yml files.
This feature was created to add standard iocs to the epics-in-docker image which can be exported as no-build. It is now possible to pass a flag (-i) to install_from_github() and install_module() to generate the envPaths file for the ioc. Since these functions are no longer only about modules, it was also necessary to change some function names.
ericonr
force-pushed
the
opcua
branch
2 times, most recently
from
40e00b5 to
030afce
Compare
The module was installed in the modules directory, because there can be other IOCs that depend on the ether_ip module, even though it ships an IOC of its own.
A new install script was added for it because it isn't supported for all our current platforms, due to being prebuilt, and includes extra build steps for that same reason. Its install location follows the same reasoning as ether_ip.
