New im6k4 ppm adc and updating im4k2 to use ppm from new cc #80
Conversation
|
@ZLLentz I talked with Josue and he mentioned you wrote the code the custom IM4K2_PPM function blocks sans the analog input FB. In the PPM and PPM_PowerMeter from cc, there are two instances of FB_CC_TempSensor which is a grandchild class of FB_TempSensor with some fast fault stuff added (from a brief glance). The custom IM4K2 PPM function blocks replaced these with FB_ThermoCouple which is a class that says it is deprecated and should be replaced with FB_TempSensor https://github.com/pcdshub/lcls-twincat-general/blob/bd00e4e3571e69ea9c7803d900ae5b21f35a24d2/LCLSGeneral/LCLSGeneral/POUs/Hardware/FB_ThermoCouple.TcPOU#L6 Do you remember if there is any reason why it wouldn't be ok for IM4K2 to use an FB_CC_TempSensor, like if someone intentionally did not want it to throw temperature-related faults? I am asking because by switching IM4K2 to the common PPM class from the common components pull request that goes with this, IM4K2 would be using FB_CC_TempSensor like the rest of the ppms and I want to make sure that isn't conflicting with an intentional prior decision. |
|
After testing with Philip today, the readout voltage looks good. I had to make two changes to fTermMax and fTermMin which were to set fTermMin to zero and multiply fTermMax by 1000. Multiplying by 1000 was just so the value was mV instead of V, like all the other imagers. Setting fTermMin to zero is for a weirder reason. This page gives a description of how to generally map a value in one range to an analogous value in another range. In the majority of places where FB_AnalogInput was used , most set fTermMin to zero. The only non-zero examples I saw were flow meters setting it to POSITIVE 0.050472, the custom IM4K2 AnalogInput, and FB_RCI_Analog_Input used in https://github.com/pcdshub/lcls-plc-crixs-motion/blob/8f1f4b4dacca1c47fb2db02fb7274c40a8eff9cb/lcls-plc-crixs-motion/PLC_CRIX_MOT/POUs/Assemblies/PRG_RCI.TcPOU#L51. The last two only use fTermMin for scaling and did not add or subtract them anywhere. I think that we can slightly alter the way that FB_AnalogInput works to match the rosettacode page I linked which would be nice if anyone ever uses a range that goes negative in the future, or I can continue to use this "hack" of setting fTermMin. I haven't thought about it enough to be confident, but I think that having a nonnegative value for fTermMin is not currently a problem, which would be good for the flow meters that currently set it to +0.050472. I think there is only a problem if fTermMin and fTermMax have different signs. I wasn't sure if I should have posted this in my general pr or here but I thought the power meter context made it reasonable to put here. I can paste it there too if you want. Sorry for this long comment, but I am trying to clearly describe what I think is a bug. Let me know if what I am saying does not make sense and I can try to clarify. |
IIRC I put this together in 20 mins on a support call, so I don't really remember much. You should feel empowered to replace what I had with more thoroughly thought-through and implemented ideas. |
I think you're absolutely correct that this is a bug in the analog conversion function block. I'm happy to review a PR with a fix. I'm also interested in an API change that makes it less confusing to set up- I'd really like someone to be able to read that table in the docs and put that information directly into the inputs, for example. |
Description
Copy of #79
I had some trouble scanning io and ended up starting from a clean branch off main, and making my edits after adding the new slice to Device. Sorry for the reviewing inconvenience. All the changes I made in the last pull request were directly copied over to this one after scanning the new io in.
Related to pcdshub/lcls-twincat-common-components#79
Related to pcdshub/lcls-twincat-general#90
Meant to use the new FB_PPM functions blocks from common-components instead of custom function blocks to support high resolution adcs such as the EL3602 for voltage measurements for IM4K2 instead of copying the slightly modifying the function blocks from common components
Adding a high resolution adc to IM6K2 like IM4K2
Motivation and Context
To add another high resolution adc in rix
Testing changes to general and common-component libraries
Making IM4K2 (and future ppms with resolutions adcs) easier to support without custom function blocks
How Has This Been Tested?
The new slice for IM6K2 has been installed and scanned, and the new code is running. Tomorrow we are going to test if the calculated voltage looks reasonable using a mounted light that is powerful that the power meter should register a voltage. IM4K2 and the other PPMs look like they are running reasonably, which is a good sign that the changed to general and common-components are working.
Where Has This Been Documented?
N/A
Screenshots (if appropriate):
Pre-merge checklist
Always Newestpre-commit(alternativelypre-commit run --all-files)