Skip to content

Conversation

@NatLeung96
Copy link
Collaborator

PV parser has been updated to correctly parse PV formulae and to parse PV starting with "=" to start with "eq://". "eq://" has been added to list of plugins in store.ts to enable formulae subscribe requests.

@NatLeung96 NatLeung96 requested a review from rjwills28 May 13, 2025 10:29
@NatLeung96 NatLeung96 linked an issue May 13, 2025 that may be closed by this pull request
Copy link
Collaborator

@rjwills28 rjwills28 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This look really good! I've tested it out and all works really nicely. I just noticed one very minor point that could be addressed before merging - see inline comment.

src/types/pv.ts Outdated
// after the PV object has been created.
if (this.name.includes(PV.DELIMITER)) {
// Need to make sure that the PV.DELIMITER is not associated with a nested PV.
if (this.name.includes(PV.DELIMITER) && !this.name.includes("`")) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor point - it looks like you can also create a formula using a single quote (i.e. ') as well as with a backtick, e.g. =3+4*'sim://ramp'. This works in Phoebus and PVWS so maybe we could just add an addition && to check that it also doesn't include a '?

@NatLeung96
Copy link
Collaborator Author

I added the check for single-quotes and I added an else-if for "eq://" in case the name is substituted with another PV formula.

@NatLeung96 NatLeung96 requested a review from rjwills28 May 13, 2025 12:35
@NatLeung96
Copy link
Collaborator Author

Also made that else-if check for "=".

Copy link
Collaborator

@rjwills28 rjwills28 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great - thanks for making those changes.

@NatLeung96 NatLeung96 merged commit 9be55c2 into master May 13, 2025
2 checks passed
@NatLeung96 NatLeung96 deleted the 81-pv-formulae-not-supported branch May 13, 2025 15:09
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.

PV formulae not supported

3 participants