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

fix(wizards/foundation): Added missing DAI bTypes #1320

Merged
merged 1 commit into from
Sep 12, 2023
Merged

fix(wizards/foundation): Added missing DAI bTypes #1320

merged 1 commit into from
Sep 12, 2023

Conversation

jarradraumati
Copy link
Contributor

Closes #1297

  • Added missing DAI bTypes from IEC 61850-6 ed. 2.1
  • Updated test snapshots to include added bTypes
  • Octet assumed to be no zero padding, no space hexadecimal string value

I think the treatment of Octet bTypes needs some consideration. However, this PR does allow for DAI of bType ObjectReference to be edited as requested.

I would appreciate a review, and further refinement if required.

* Added missing DAI bTypes from IEC 61850-6 ed. 2.1
* Updated test snapshots to include added bTypes
* Octet assumed to be no zero padding, no space hexadecimal string value
@pascalwilbrink
Copy link
Member

@JakobVogelsang could you take a look at this?

Copy link
Collaborator

@JakobVogelsang JakobVogelsang left a comment

Choose a reason for hiding this comment

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

In general LvGTM. Thank you especially for the well written issue linked to the PR.

I have just a little comment. I would have expected to see a regression test showing grRef data attributes are not disabled and have an edit button next to it as this was the starting point for this PR in the first place. But this also is just a comment that does not need to be resolved as we have not done so for other bType's as well.

@jarradraumati
Copy link
Contributor Author

I have just a little comment. I would have expected to see a regression test showing grRef data attributes are not disabled and have an edit button next to it as this was the starting point for this PR in the first place. But this also is just a comment that does not need to be resolved as we have not done so for other bType's as well

Ah yes I had thought about doing that. There's also a case for ensuring that bTypes we don't want to allow editing are disabled. But I was struggling to get the tests working correctly in my environment (something I need to look into), and the day was dragging on... Perhaps for the next PR.

@pascalwilbrink
Copy link
Member

@jarradraumati, Thank you very much for the contribution :)
We'll merge it for now (there is a big PR coming up where the folder structure is changed). I'll update that branch with main

@pascalwilbrink pascalwilbrink merged commit 0bff5aa into openscd:main Sep 12, 2023
1 check passed
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.

IED editor: Allow editing and instantiation of all values
4 participants