-
Notifications
You must be signed in to change notification settings - Fork 31
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
ioc: improve long string detection #47
Conversation
@mdavidsaver thanks, this fixes my immediate problem, reading I'll consider what the best mix of DiamondLightSource/pythonSoftIOC#132 and DiamondLightSource/pythonSoftIOC#133 is to merge when there's a new release of pvxslibs to point at |
✅ Build pvxs 1.0.898 completed (commit 324c44af22 by @mdavidsaver) |
Unless I hear cries of horror from @anjohnson this week, I'll merge and release. |
I haven’t checked the code since I’m only using an iPad right now, but I believe it’s legal to follow a Is it not possible to support |
Probably, although this does raise the issue of retraining users not to append I have also found myself considering always mapping |
Personally I would prefer this. For my 3 use cases it would be great if pvget worked without the
I would be happy to move any of my usage of long strings in a
For |
How about an opt-in for this, like |
f80c194
to
3a8c882
Compare
✅ Build pvxs 1.0.900 completed (commit f034043f70 by @mdavidsaver) |
3a8c882
to
b7f680b
Compare
I've rewritten around this idea. I think the changes in the test code illustrate things. Reading With this change, access to
This is a point...
I think I have figured out a way to make this work without adding another
|
@anjohnson I've opted for "mangling" the Also, at the moment I'm the "mangling" logic doesn't test |
I haven't tried to compare the code, but I have an impression that this is cleaner than the previous version (although I don't remember looking at that too closely). I was hoping we could offer a solution such that users at PVA-only sites won't need to worry about Could adding an alternate My one concern is that I assume adding |
How about adding a It will also be necessary to give some indication that this promotion has happened. This is really the missing piece in I was thinking to add a flag like
Yup. I didn't think of this back when Of course aSub is an extra special case, which I'm inclined to ignore for the present. |
✅ Build pvxs 1.0.902 completed (commit efaff700b4 by @mdavidsaver) |
b7f680b
to
0b3f7ec
Compare
I have updated things such that the |
✅ Build pvxs 1.0.906 completed (commit cd06817c7e by @mdavidsaver) |
Partially replicate the '$' handling logic of dbChannelCreate() in the case where no '$' was provided.
0b3f7ec
to
a02b6e9
Compare
@coretl If you have time, can you take another look at this? Despite 1.2.1 only being 6 days old, after working through |
❌ Build pvxs 1.0.909 failed (commit daf36adbac by @mdavidsaver) |
✅ Build pvxs 1.0.909 completed (commit daf36adbac by @mdavidsaver) |
I've installed the python3.10 linux wheel from https://github.com/mdavidsaver/pvxs/actions/runs/5304865866 and it seems to work fine for me, no |
Attempt to improve automatic handling of "long strings". At least to handle the
$
case.I had hoped that I could come up with a way to detect the effects of
$
or similar server side filters by inspecting the types stored in thedbChannel
anddbAddr
structs. However, I've almost reached the point of giving up. Between the mangling done indbChannelCreate()
, and in variousrset::cvt_dbaddr()
, I don't think this can reliably be done without extendingdbChannel
.And there are also cases like
waveformRecord
withFTVL=CHAR
, where.VAL$
has never been supported because the underlying field type isn'tDBR_STRING
.@anjohnson @coretl fyi.