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 PV inverter error display and clean up product IDs #1320

Merged
merged 2 commits into from
Jul 15, 2024

Conversation

blammit
Copy link
Contributor

@blammit blammit commented Jul 12, 2024

No description provided.

Use the same error text that is shown when clicking on a PV Inverter
from the Device List.

Fixes #1303
Copy link
Contributor

@DanielMcInnes DanielMcInnes left a comment

Choose a reason for hiding this comment

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

lgtm, nits

class ProductInfo : public QObject
{
Q_OBJECT
QML_NAMED_ELEMENT(ProductInfo)
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just QML_ELEMENT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh true, the QML_NAMED_ELEMENT wasn't necessary. Fixed.

~ProductInfo() override {}

enum ProductId_Alternator {
ProductId_Wakespeed = 0xB080, // VE_PROD_ID_WAKESPEED_WS500
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe prefix the enum entry with the enum title, eg. ProductId_Alternator_Wakespeed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, some of the others didn't have a clear grouping, so I added Product_Misc and put those enums there.

Q_ENUM(ProductId_Genset)

Q_INVOKABLE bool isGensetProduct(int productId) {
static const QVector<int> productIds = {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe a switch/case instead of a QVector with static lifetime?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, much nicer. Fixed, thanks.

@blammit blammit force-pushed the blam/pv-inverter-error branch 2 times, most recently from 092ee25 to 1fe2bb4 Compare July 15, 2024 05:08
@blammit blammit force-pushed the blam/pv-inverter-error branch from 1fe2bb4 to 09a1485 Compare July 15, 2024 05:11
@blammit blammit merged commit 62b8be5 into main Jul 15, 2024
1 check passed
@blammit blammit deleted the blam/pv-inverter-error branch July 15, 2024 06:05
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.

2 participants