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

Fix enum undef ts bug35 #43

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

Conversation

bhill-slac
Copy link
Contributor

When support for propEventCB was added to the gateway, it subscribed to DBR_CTRL_* but calls both runDataCB and runValueDataCB. Since the DBR_CTRL_* events have zero in the timestamp fields and the value_data_callbacks assign the gdd from the DBR_CTRL* event to the gatePv value gdd, it can step on the timestamp from the DBR_TIME_* event.

I suspect the code in gatePvData::propEventCB was ignoring the first propEvent to avoid this bug but some cases get through and set the timestamp fields to zero, in particular mbbi and mbbo records.

This fix creates to subcriptions for DBE_PROPERTY events, one DBR_CTRL* subscription w/ propDataCB, and a DBR_TIME* subscription w/ propEventCB.

@bhill-slac bhill-slac force-pushed the fix-enum-undef-ts-bug35 branch from ef931b8 to 12002c9 Compare May 24, 2022 23:36
@ralphlange
Copy link
Contributor

This pull request fails the tests on all different architectures.
Main issue, which seems directly related to the changes in this PR:

make[3]: Entering directory '/home/runner/work/ca-gateway/ca-gateway/testTop/pyTestsApp/O.linux-x86_64'
  ======================================================================
  FAIL: Monitor PV (imitating CS-Studio) through GW - change value and properties directly - check CTRL structure consistency
  ----------------------------------------------------------------------
  Traceback (most recent call last):
    File "/home/runner/work/ca-gateway/ca-gateway/testTop/pyTestsApp/TestCSStudio.py", line 93, in testCSStudio_ValueAndPropMonitor
      self.assertTrue(are_diff == False,
  AssertionError: False is not true : At update 3 (change value), received structure updates differ:
  	Element 'value' : GW has '0.0', IOC has '10.0'
  	Element 'ftype' : GW has '34', IOC has '20'
  	Element 'status' : GW has '17', IOC has '4'
  	Element 'severity' : GW has '3', IOC has '1'
  	Element 'timestamp' : GW has '631152000.0', IOC has '1653[43](https://github.com/epics-extensions/ca-gateway/runs/6583342460?check_suite_focus=true#step:8:44)5554.5943'
  	Element 'posixseconds' : GW has '6311[52](https://github.com/epics-extensions/ca-gateway/runs/6583342460?check_suite_focus=true#step:8:53)000.0', IOC has '16[53](https://github.com/epics-extensions/ca-gateway/runs/6583342460?check_suite_focus=true#step:8:54)4355[54](https://github.com/epics-extensions/ca-gateway/runs/6583342460?check_suite_focus=true#step:8:55).0'
  	Element 'nanoseconds' : GW has '0', IOC has '[59](https://github.com/epics-extensions/ca-gateway/runs/6583342460?check_suite_focus=true#step:8:60)4300114'
  
  ======================================================================
  FAIL: DBE_PROPERTY monitor on an ai - value changes generate no events; property changes generate events.
  ----------------------------------------------------------------------
  Traceback (most recent call last):
    File "/home/runner/work/ca-gateway/ca-gateway/testTop/pyTestsApp/TestDBEProp.py", line [60](https://github.com/epics-extensions/ca-gateway/runs/6583342460?check_suite_focus=true#step:8:61), in testPropAlarmLevels
      self.assertTrue(self.eventsReceivedGW == 1, 'GW events expected: 1; received: ' + str(self.eventsReceivedGW))
  AssertionError: False is not true : GW events expected: 1; received: 2
  
  ----------------------------------------------------------------------

Please either fix the changes or the test.

@bhill-slac
Copy link
Contributor Author

Looks like the test just needed to adjust the expected eventsReceived counts.
This is expected as the gateway w/ this branch is doing two subscriptions instead of one.

@ralphlange
Copy link
Contributor

Thanks a lot, Bruce!
I had an idea but was not 100% sure if the code or the test needed changing.

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.

2 participants