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

Possibility to add indicators on busbarsection #354

Merged
merged 23 commits into from
Mar 3, 2022

Conversation

tadam50
Copy link
Contributor

@tadam50 tadam50 commented Feb 21, 2022

Please check if the PR fulfills these requirements (please use '[x]' to check the checkboxes, or submit the PR and then click the checkboxes)

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem ?
Fixes #311

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
feature

What is the current behavior? (You can also link to an open issue here)
No indicator provided on busbar section

What is the new behavior (if this is a feature change)?
Be able to add indicator on busbar section

Does this PR introduce a breaking change or deprecate an API?
No

@tadam50 tadam50 requested a review from flo-dup February 21, 2022 14:42
@tadam50 tadam50 self-assigned this Feb 21, 2022
Copy link
Contributor

@flo-dup flo-dup left a comment

Choose a reason for hiding this comment

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

Great! Similarly to the FeederInfo you can now add a BusInfo given by the LabelProvider to draw something in that new cell

@tadam50 tadam50 force-pushed the issue_311_add_lack_voltage_information_on_bus branch from ae96230 to d7e895b Compare February 22, 2022 14:16
@tadam50
Copy link
Contributor Author

tadam50 commented Feb 23, 2022

Indicators drawn with bus style as following :
TestCase15GraphWithVoltageLackInfo

TestCase15GraphWithVoltageLackInfoTopological

Copy link
Contributor

@flo-dup flo-dup left a comment

Choose a reason for hiding this comment

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

Great job! I think we can now go a bit further, and always add a cell on the left but also one on the right. Then an enum parameter on the BusInfo will tell us if we need to draw it on the left or right side of the bus.

@@ -34,6 +34,8 @@

private final boolean exceptionIfPatternNotHandled;

private final boolean voltageIndicatorOnBus;
Copy link
Contributor

Choose a reason for hiding this comment

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

voltageIndicator is only one possible use for busInfo, so we should keep that keyword for the test only

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe addCellForBusInfo, what do you think?

GraphMetadata metadata,
DiagramLabelProvider initProvider,
DiagramStyleProvider styleProvider) {
Optional<BusInfo> busInfo = initProvider.getBusInfo(busNode);
Copy link
Contributor

Choose a reason for hiding this comment

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

you can remove busInfo variable, you don't need it (use info in the lambda instead of busInfo.get())

@flo-dup flo-dup marked this pull request as ready for review March 3, 2022 08:40
tadam50 added 21 commits March 3, 2022 10:48
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
…eft sode of busbar

Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
tadam50 added 2 commits March 3, 2022 10:48
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
Signed-off-by: Thomas ADAM <tadam@silicom.fr>
@tadam50 tadam50 force-pushed the issue_311_add_lack_voltage_information_on_bus branch from 71a1123 to 6d5c8f8 Compare March 3, 2022 09:49
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 3, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

99.1% 99.1% Coverage
0.0% 0.0% Duplication

@flo-dup flo-dup merged commit 6df567f into main Mar 3, 2022
@flo-dup flo-dup deleted the issue_311_add_lack_voltage_information_on_bus branch March 3, 2022 13:27
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.

Possibility to add indicators on busbarsection
2 participants