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

Miketrahearn/1239 align genset settings and control card #1812

Conversation

MikeTrahearn-Qinetic
Copy link
Contributor

No description provided.

@MikeTrahearn-Qinetic
Copy link
Contributor Author

@blammit should I be considering using your new work in #1810 for this ticket?

@MikeTrahearn-Qinetic MikeTrahearn-Qinetic force-pushed the miketrahearn/1239-align-genset-settings-and-control-card branch from 62ef855 to 3fe657d Compare January 6, 2025 01:04
Copy link
Contributor

@blammit blammit left a comment

Choose a reason for hiding this comment

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

Can you add a reference to the issue number in each commit?

components/PageGensetModel.qml Outdated Show resolved Hide resolved
data/common/Generator.qml Outdated Show resolved Hide resolved
data/common/Generator.qml Outdated Show resolved Hide resolved
@blammit
Copy link
Contributor

blammit commented Jan 6, 2025

@blammit should I be considering using your new work in #1810 for this ticket?

No, that's not necessary; that PR is about finding the generator for a genset service, which you don't need to do here.

@MikeTrahearn-Qinetic MikeTrahearn-Qinetic force-pushed the miketrahearn/1239-align-genset-settings-and-control-card branch 2 times, most recently from b62d33d to b4d116a Compare January 6, 2025 06:24
Copy link
Contributor

@blammit blammit left a comment

Choose a reason for hiding this comment

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

Could you also patch data/mock/GeneratorsImpl.qml to set Enabled=1 for the mock generators? Like this:

diff --git a/data/mock/GeneratorsImpl.qml b/data/mock/GeneratorsImpl.qml
index 642df01d..a5c715bf 100644
--- a/data/mock/GeneratorsImpl.qml
+++ b/data/mock/GeneratorsImpl.qml
@@ -33,6 +33,7 @@ QtObject {
                                _customName.setValue("Start/Stop generator")
                                _state.setValue(VenusOS.Generators_State_Running)
                                _runningBy.setValue(VenusOS.Generators_RunningBy_Soc)
+                               Global.mockDataSimulator.setMockValue(generator.serviceUid + "/Enabled", 1)
                                setAutoStart(true)
                        }

Otherwise they won't show up in the generator cards.

Aside from that, LGTM. I spotted one other thing - could you add the common dialog component as well? As per comment.

@MikeTrahearn-Qinetic MikeTrahearn-Qinetic force-pushed the miketrahearn/1239-align-genset-settings-and-control-card branch from b4d116a to b6aa09b Compare January 8, 2025 00:15
@MikeTrahearn-Qinetic MikeTrahearn-Qinetic merged commit 53888a0 into main Jan 8, 2025
2 checks passed
@MikeTrahearn-Qinetic MikeTrahearn-Qinetic deleted the miketrahearn/1239-align-genset-settings-and-control-card branch January 8, 2025 00:52
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.

3 participants