-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add three new ZMQ publishers for TP results / Retrieve TP results #2230
base: main
Are you sure you want to change the base?
Conversation
33d4401
to
716f021
Compare
@t-b Feedback request for general design improvements. |
General approach looks good. Two comments:
|
96f6592
to
9c33299
Compare
Git review: ef3f1d2 (Util: Add two conversion function for DA/AD unit string return, 2024-08-22) Good idea!
27dac75 (TP: Add more information that is transferred to the TP analysis thread, 2024-08-22) Nice! 6368bbb (PUB: Add four zeromq publishers for TP data, 2024-08-22)
The problem is likely that FetchPublishedMessage only works with a single filter. So we might need to teach FetchPublishedMessage to check fo multiple filters and return then multiple messages.
9c33299 (TP: Added two functions that allow to retrieve info about TPs, 2024-08-22) commit message:
|
|
ef494e7
to
e82cd88
Compare
@MichaelHuth Please rebase |
e82cd88
to
82d748d
Compare
28dcb99
to
e86dda8
Compare
CI is failing. Review: Looks all good. 31d1771 (Getters: Cleanup of GetTPStorage wave upgrade, 2024-08-25) Now looking at this change in isolation, I realize that this is not doing the right thing. Consider a wave upgrade for version 13. The old code set layers 24 to 30. But 2802321 (PUB: Preparation to add four zeromq publishers for TP data, 2024-08-25) TestTPPublishing_REENTRY: Instead of Make/FREE/T filters = {ZMQ_FILTER_TPRESULT_NOW, ZMQ_FILTER_TPRESULT_1S, ZMQ_FILTER_TPRESULT_5S, ZMQ_FILTER_TPRESULT_10S} you can just write WAVE filters = DataGenerators#PUB_TPFilters() 0057895 (TP: Rename TP_GetStoredTPs to TP_GetConsecutiveTPsUptoMarker, 2024-08-25) We nowadays have IsValidHeadstage. e86dda8 (TP: Added two functions that allow to retrieve info about TPs, 2024-08-22) Looks good! |
3b191f9
to
c1fc4e9
Compare
c1fc4e9
to
7030c77
Compare
We return a wave reference wave from the get tp function and I was curious if we are handling recursive wave references correctly, yes we do. Function Dostuff()
make/o/WAVE/N=1 data
make/o/WAVE/N=1 elem1
make/o/WAVE/N=1 elem2
data[0] = elem1
elem1[0] = elem2
print data, elem1, elem2
print zeromq_test_serializewave(data)
End
gives
|
7030c77
to
9a5a3ff
Compare
Fixed conflicts. |
@campagnola, can you confirm that this PR has the necessary functionality to pass TP data from MIES to ACQ4? |
I will work with Ben to get this tested. Neither of us is very familiar with Igor, though; is the protocol documented somewhere? |
To test this I would do:
In a python console:
python example code is at https://github.com/AllenInstitute/ZeroMQ-XOP/tree/main/examples#python-client.
We already do have tests that things work like we think it should, so this is more a request to check that it works like you think it should. The idea of the PR is that programs interfacing with MIES (aka ACQ4) don't need to poll the TP values with Another thing we added is the possibility to query the stored TPs via |
9a5a3ff
to
35c5eda
Compare
rebased against latest main. |
d1642af
to
e1a6425
Compare
Fixed conflicts. |
e1a6425
to
a64fd66
Compare
Fixed conflicts. |
Added GetADChannelUnit, GetDAChannelUnit that return the unit string depending on clamp mode. Adapted GetChanAmpAssignUnit to use the new functions.
- extended the TPAnalysisInput structure This is a preparation commit for adding zeromq publishers that include some of that information.
- the data is published from the TP analysis thread including additional information available in the thread through the previous commit. - The additional values are also returned by the thread and collected in the async buffer as well then in TPResult and in TPStorage. - The involved waves and their respective getters were adapted with new elements that the additional data can be stored. - As most of the elements store the same information, thus a constant was introduced with a dimension label list that is used as helper for the wave creation in the getter functions.
- The four publishers publish the same json, just with a different period. There is a filter for live, 1s, 5s and 10s publishing interval. - See PUB_TPResult for JSON description - publisher is called from TP_TSAnalysis thread
This prevent misleading naming and it more fitting to the functionality the function actually implements
Added TP_GetStoredTP and TP_GetStoredTPsFromCycle that allow to get information about a TP by tpMarker or TPs by cycle id and headstage. - both functions allow to recreate the DA wave for the TPs with the flag includeDAC - the returned data includes the AD, DA data as well as metadata for each returned TP (from TPStorage). - These TP functions use the same TP utility function.
With a running TP adding zeromq publishing messages for each TP it appears that we have to look through more than the last 100 messages after a test to find the requested. - split logic into two parts: either read out upto 10000 existing messages or wait up to 10 seconds (100 trys with 100 ms sleep) if no message is available
a64fd66
to
fc20936
Compare
Publishers added:
testpulse results live
testpulse results 1s update
testpulse results 5s update
testpulse results 10s update
The TP results from TP analysis are packed in a JSON in TP_TSAnalysis and published to ZMQ.
The time information when the next publishing is due is stored in the TUFXOP.
close #2157