-
Notifications
You must be signed in to change notification settings - Fork 4
feat(hash): dereference FCDA with doName attribute #73
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
base: main
Are you sure you want to change the base?
Conversation
97eacdd to
cba4184
Compare
We do not yet dereference FCDAs that lack a doName attribute and thus do not support GSSE yet.
danyill
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the opportunity to comment on this PR.
I have a question and a few comments.
| `:scope>LDevice[inst="${ldInst}"]>LN0[prefix="${prefix}"][lnClass="${lnClass}"], | ||
| :scope>LDevice[inst="${ldInst}"]> LN[prefix="${prefix}"][lnClass="${lnClass}"][inst="${lnInst}"]`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because (regrettably) so many FCDA attributes are optional and because the standard is unclear we have to deal with another special case.
In Ed 2.1 for prefix: "Prefix identifying together with lnInst and lnClass the LN where the DO resides; optional, default value is the empty string". Often we interpret this as prefix="" or prefix missing entirely as equivalent.
Prefix is optional within tLN with a default of "" so I think this is a legitimate case we should consider and for that I am sorry 😉 (and the situation is worse with ExtRef elements!)
| } else if (fc) { | ||
| references.push( | ||
| ...Array.from(doElement.querySelectorAll(`:scope>DA[fc="${fc}"]`)), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps a silly question:
If I had an FCDA which only looked at at a DO, would the name of the DO somehow be included here, is this semantic? i.e. two boolean values with the same structure are the same even if the names are different?
This seems like some kind of duck typing vs. structural typing debate.
Somehow in my mind I feel the name of a data object is structural in a different way to, say, the name of an IED or DataSet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like some kind of duck typing vs. structural typing debate.
Somehow in my mind I feel the name of a data object is structural in a different way to, say, the name of an IED or DataSet.
I think I think this is because these are defined as part of the standard in Parts 7-3 and 7-4 and have a "well defined meaning". But up for discussion 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very good point. If structural typing is desired, DO/DA names should be excepted from the base filter's exclude list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we could ask @JakobVogelsang for a comment on this topic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be clear: Once #47 is resolved by @stee-re 's current work, whether we do structural typing or duck typing is up to the distributor - or even up to the end user if they're willing to edit the base filter. Jakob's input would still be valuable for deciding which default base filter we want to deliver out of the box.
Closes #69 , which would be nice. However, we do not yet dereference FCDAs that lack a doName attribute and thus do not support GSSE yet. This we would have to live with for the time being.
Also, does not come with automated tests yet, so needs to be manually tested (ideally by @danyill ) that FCDAs catch all relevant DAs, DAIs, and so on. Constructing sufficient test data is quite a chore, so I'd suggest doing that next month, after the Transpower MVP is in production.