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

Unable to monitor #1

Closed
fonty422 opened this issue Jul 5, 2023 · 11 comments
Closed

Unable to monitor #1

fonty422 opened this issue Jul 5, 2023 · 11 comments

Comments

@fonty422
Copy link

fonty422 commented Jul 5, 2023

Running the default example:

async with pv.monitor() as mon:
    async for value in mon:
        print(value)

yields an error:

AttributeError: 'Pv' object has no attribute 'monitor'

Also, is there any possibility of adding connection callbacks?

@fonty422
Copy link
Author

fonty422 commented Jul 5, 2023

While at it, the basic await pv.get() appears to return None, while a classic pyepics get() on the same PV definitely give a value

@agerasev
Copy link
Owner

agerasev commented Jul 8, 2023

Running the default example: ... yields an error: ...

Sorry, I forgot to update README to version 0.2. Updated now, thanks for pointing out.

In 0.2 there are two distinct Pv types: monitoring and non-monitoring. Here is a new monitoring example:

from pyepics_asyncio import PvMonitor

pv = await PvMonitor.connect("pvname")
async for value in pv:
    print(value)

@agerasev
Copy link
Owner

agerasev commented Jul 8, 2023

Also, is there any possibility of adding connection callbacks?

You can create basic pyepics Pv with desired parameters (e.g. connection callback) and then wrap it in pyepics_asyncio Pv or PvMonitor:

def callback():
    ...

raw = epics.PV(
    "pvname",
    form="native",
    connection_callback=callback,
)

pv = Pv(raw)

@agerasev
Copy link
Owner

agerasev commented Jul 8, 2023

While at it, the basic await pv.get() appears to return None, while a classic pyepics get() on the same PV definitely give a value

It's interesting. Pv.get simply calls epics.Pv.get in executor and returns its result.
Could you show your code for both cases?

@fonty422
Copy link
Author

Thanks heaps for the update. Here's the example you requested

from pyepics_asyncio import Pv, PvMonitor
from epics import PV

pv = await Pv.connect("SR11BCM01:CURRENT_MONITOR")
pv_ = PV("SR11BCM01:CURRENT_MONITOR")

print(await pv.get())
print(pv_.get())

>> None
>> 200.19154430039114

I'll play around with the PvMonitor today and let you know if there are any further questions.

@fonty422
Copy link
Author

fonty422 commented Jul 13, 2023

It's still unhappy with things. I did also update to version 0.2.1 from 0.2.0 (which I assume was only the recent readme updates).

pv_async = await PvMonitor.connect("SR11BCM01:CURRENT_MONITOR")
async for value in pv_async:
    print(value)

with the error being:

----> 1 pv_async = await PvMonitor.connect("SR11BCM01:CURRENT_MONITOR")
AttributeError: '_ConnectMonitor' object has no attribute 'name'

EDIT
You can't monitor a PV that is referenced elsewhere? So if I have pv = await Pv.connect("SR11BCM01:CURRENT_MONITOR") or pv_ = epics.PV("SR11BCM01:CURRENT_MONITOR") I can't also have pv_async = await PvMonitor.connect("SR11BCM01:CURRENT_MONITOR")? If I only have the pvMonitor it works as expected.

The following still results in a None:

from pyepics_asyncio import Pv, PvMonitor
pv = await Pv.connect("SR11BCM01:CURRENT_MONITOR")
print(await pv.get())

@fonty422
Copy link
Author

Ah, the Pv class doesn't return the awaited value.
This line needs a return statement added

@agerasev
Copy link
Owner

Okay, I've write some tests with dummy IOC and found some issues:

  1. Pv.get method wasn't returned value from epics.PV.get(). But I reworked it even further. PyEpics has no way to set callback for PV.get, so now get enables monitor and disables it after obtaining first value instead of using blocking PV.get.
  2. Connection callback may fire before _ConnectMonitor future fully initialized. This leads to object has no attribute 'name' error. Fixed by calling epics.PV.__init__ in-place only after future initialization.
  3. It's not a bug, but undesirable behavior. PvMonitor previously stored all values it received, that caused memory overfill when being connected to frequently updated PV for a long time. Now monitor stores at most two values: last received and last unread.

Thank you for your assistance! It's nice to see that this library is used by someone.

@fonty422
Copy link
Author

fonty422 commented Jul 14, 2023

No, thank you for doing what I was about to try and do but wasn't sure I had the skill.
We have an asyncio application we're developing and I was just getting the event loop then using call_soon_threadsafe() on a basic callback to call another function, but your solution is much prettier.

I thinking I might try add a _Notify method to the PvMonitor so if that is not None, then it triggers the async call to perform some other action (to send to a kafka service, in our case). Would you prefer to keep that separate to this and keep the library clean and then extend this myself separately?
Oh, and we think the form option should be enabled as an option too, which can default to 'native' so we can get other parameters too.

@agerasev
Copy link
Owner

I thinking I might try add a _Notify method to the PvMonitor so if that is not None, then it triggers the async call to perform some other action (to send to a kafka service, in our case). Would you prefer to keep that separate to this and keep the library clean and then extend this myself separately? Oh, and we think the form option should be enabled as an option too, which can default to 'native' so we can get other parameters too.

Feel free to open pull requests to this repo. If you need a feature, someone else may need it too.

As for notification callback, maybe it's better to simply spawn a new task that iterates over monitor? It seems to be more async/await-way rather than callbacks. Or I missing something?

@fonty422
Copy link
Author

Ah, yes, you're right, the notify is more an async/await thing so should be separate.
I do have a few small ideas to make the package more flexible, but they're basically just allowing the user the option to tailor the request to include the full gamut of options from pyepics which can all default to some fall-back if they're not supplied.
I might pull and if you like the changes, then you're free to accept them.

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

No branches or pull requests

2 participants