-
Notifications
You must be signed in to change notification settings - Fork 2
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
base: add SNMP module #67
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we have discussed in person, there are two repositories for SNMP at slac-epics:
Both of them seem to implement the support for SNMP. Taking a brief look at them, the latter has been changed on GitHub more recently (6 months versus 3 years ago), but supports less record types than the former. slac-epics/epics-snmp
also has more custom configurations in its build, which would require us to override them.
Considering that:
- we have tested with slac-epics/snmp and it has worked just fine for the simple IOC we need;
- the IOC is simpler with the mbbo records we would lose with epics-snmp;
- the build required no custom handling from our part
IMHO, we can stick with it, at least for now. If it turns out to be deprecated by SLAC or the other becomes more appealing, we can reconsider this choice.
I've complemented the commit message with the relevant points regarding the choice of using slac-epics/snmp. Can you double check if everything is okay with it? |
I've asked in tech-talk about the other modules and for recommendations to see if this is indeed the best choice. Let's see the community point of view of this. |
d9226a7
to
8323d79
Compare
Now lnls-get-n-unpack function downloads and extracts .zip files as well. File extension (case insensitive) is used to check the archive type, as it should provide a reasonable file type hint. A more robust approach would be to use `file -b --mime-type`, but this would introduce yet another dependency in both build and runtime images. Let's assume file extension conventions are followed instead. Overwrite option (-o) and quiet mode (-q) are enabled, following the default behavior from `tar`, and avoiding interactive prompts. Ownership is not kept by default, so no extra flag is needed. There is no need to install unzip in IOC and musl-based image, since busybox(1) already provides it. In the runtime image, we only need to symlink unzip, so that it becomes available in the PATH. Co-authored-by: Henrique F. Simoes <henrique.simoes@lnls.br>
Over time, several SNMP device support have been implemented, most (or all) of them derived from DESY's implementation, including one from Sheng Peng, Diamond and NSCL/FRIB. By 2013, most of community members were willing to test NSCL/FRIB's implementation or to switch to it [1]. Advantages by then were: - no memory leak (opposed to Sheng Peng's implementation) - easier to work with (compared to original DESY implementation) - supports SNMP writes (compared to Sheng's) - depends more heavily on libsnmp (compared to Diamond's implementation) Nowadays, Sheng Peng's implementation seems not to be developed anymore, as well as Diamond's. From all forks, NSCL/FRIB's and SLAC's are still around [2]. Looking through SLAC's repository history, it seems to be also based on Sheng Peng's module [3], with several changes done afterwards, mostly for patching bugs and supporting new record types. Between the two, NSCL/FRIB module [4] (also forked in slac-epics/epics-snmp [5]) has been preferred over slac-epics/snmp, since it had more features in the past and the community seems to be actively using, discussing and contributing to it recently [2]. Overall, record API is the same. For general SNMP-aware devices, changing from one implementation to the other is a matter of changing the record DTYP used, paying attention to usage of extra parsing features in INP/OUT fields and record types used. Therefore, changing the implementation in the future does have costs, but it should not be very high. Co-authored-by: Henrique F. Simoes <henrique.simoes@lnls.br> [1]: https://epics.anl.gov/tech-talk/2013/msg00537.php [2]: https://epics.anl.gov/tech-talk/2024/msg00889.php [3]: slac-epics/snmp@d82df4e [4]: https://groups.nscl.msu.edu/controls/files/devSnmp.html [5]: https://github.com/slac-epics/epics-snmp
@guirodrigueslima, can you take a look in the changes I've made? They are mostly documentation changes, apart from some changes to make I think we are good to merge after that. |
filename=$(basename $download_dir/*) | ||
|
||
if [[ ${filename,,} == *".zip" ]]; then | ||
unzip -qo $download_dir/$filename -d $dest | ||
else | ||
tar --no-same-owner -xf $download_dir/$filename -C $dest | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if we end up having to support more formats, we should switch to bsdtar+libarchive directly, instead of having to handle different tools in the script. Since busybox already provides unzip, I think it's reasonable to depend on it.
Added the SNMP module, developed by SLAC (National Accelerator Laboratory). This module will be used to develop the IOC for Aruba Switches to control PoE ports.