UI: added widget to show digital input states #1195

ahummelsberger · 2020-07-31T14:41:47Z

For testing my iot2000 implementation I created a widget to show the states of all available digital input channels.
It iterates through all enabled components that implement the DigitialInput Interface and list the states of the channels.

The design surely can be improved though.

fabianfnc · 2020-08-19T14:07:39Z

Servus Andreas,

hat es einen bestimmten Grund weshalb du nur die Read Only Channels anzeigst?

Ich melde mich die Tage noch einmal wegen dem Rest des Reviews.

ahummelsberger · 2020-08-20T08:05:11Z

Servus Andreas,

hat es einen bestimmten Grund weshalb du nur die Read Only Channels anzeigst?

Ich melde mich die Tage noch einmal wegen dem Rest des Reviews.

Hallo Fabian,

da es für die Outputs ja bereits ein Widget gibt wollte ich mich hierbei nur auf die Inputs konzentrieren, damit ich einfach sehen kann, welche Signale an den Inputs anliegen.
Ich ging davon aus, dass Inputs immer Read Only sind.

fabianfnc · 2020-08-21T12:49:27Z

Das ergibt natürlich Sinn.

Erst einmal vielen Dank für deinen Pull Request!

Bezüglich des Reviews:

Wir haben gewisse Coding Guideline für das Monitoring.
Ich weiß, dass diese nirgendwo niedergeschrieben sind, was ich aber noch nachholen werde.
(in der OpenEMS Doku (openems.io))

Also bitte nicht falsch verstehen, das Review ist keineswegs unfreundlich/böse etc. gemeint.

Icon neben der Überschrift

Als Icon bitte slot="start" color="primary" size="large" name="radio-button-off-outline" verwenden.

Widget Aufbau

Die Widgets haben immer einen 'Übersichts' und einen 'Erweiterten' (die Modal Komponente) Teil.

Bei diesem Widget würde es sich anbieten im Übersichts Widget (also das, welches oben im Screenshot zu sehen ist, also im Prinzip nur io0 bzw. den Alias) die verfügbaren Geräte/Komponenten ohne die Channels/Channelwerte anzuzeigen.

Hierbei nicht die component.id, sondern den component.alias verwenden.
Wenn kein Alias definiert ist, wird sowieso die Id angezeigt.

In der erweiterten Ansicht dann die Channelwerte als table Struktur, wie in den anderen Modal Komponenten.

Keine Farbe für den Text innerhalb der Widgets festlegen.

live.component.html Problematik

Mit der Logik in der live.component.html wird pro verfügbaren Komponente/Nature/Factory ein Widget angezeigt.

Im Widget selber jedoch sammelst du nochmal alle Komponenten.

Wenn jetzt also mehr als zwei Komponente/Natures/Factories bei einem System verfügbar wären, würden zwei Widgets mit dem selben Inhalt erscheinen.

Deshalb wird auch pro widget eine component.id an das Widget übergeben.

Das hast du auch gemacht, aber die Input property im Widget selbst nicht definiert.
(Was auch sinnfrei wäre, da du denk ich mal alle Komponenten in dem Widget sammeln willst)

Hier solltest du dafür sorgen, dass das Widget nur einmal aufgerufen wird und den Übergabeparameter rausnehmen.

ahummelsberger · 2020-08-24T08:10:56Z

Danke für das Review.
Ich werde alles entsprechend anpassen. Wir nur vermutlich 2 - 3 Wochen dauern, da ich gerade viel anderes noch am laufen habe.

fabianfnc · 2020-10-01T11:51:21Z

Ist es gewollt, dass die Property Channels noch angezeigt werden?

Ansonsten passt alles soweit.

ahummelsberger · 2020-10-02T07:00:30Z

Oha, nein eigentlich nicht.
Mir war nicht bewusst, dass es noch weitere ReadOnly Channels geben kann.
Bisher habe ich nur 'State' und '_PropertyEnabled' ausgeschlossen.
Welche Möglichkeit gibt es denn (abgesehen hard coded oder Regex) die Property Channels auszuschließen?

*underline component alias *disable property channels

fabianfnc · 2020-10-02T08:59:16Z

Habe das mit !channel.key.includes('_Property')" gelöst.

Nochmal vielen Dank für deinen Pull Request!
Ich werde das ganze jetzt dann mergen.

sfeilmeier · 2020-10-02T09:41:51Z

Zumindest solltet ihr nach dem Typ BOOLEAN filtern können, oder?
Besser als includes wäre vermutlich startsWith("_Property/")
https://developer.mozilla.org/de/docs/Web/JavaScript/Reference/Global_Objects/String/startsWith

fabianfnc

Habe das html if jetzt auf channe.value.type == 'BOOLEAN' abgeändert.

sfeilmeier

@ahummelsberger, @fabianfnc: I have some comments/doubts on this pull-request. Can you please have a look?

sfeilmeier · 2020-10-12T07:27:26Z

ui/src/app/edge/live/digitalinput/digitalinput.component.html

+            <ion-card-content>
+                <table class="full_width">
+                    <tr>
+                        <td style="width:65%" translate>Anzahl Komponenten</td>


"translate" needs an identifier for the string, right? I don't think it will work this way.

sfeilmeier · 2020-10-12T07:44:07Z

ui/src/app/edge/live/digitalinput/digitalinput.component.ts

+
+
+
+            }


Why so many empty lines?

sfeilmeier · 2020-10-12T07:44:53Z

ui/src/app/edge/live/digitalinput/modal/modal.component.html

+                        <ng-container *ngFor="let channel of (component.channels | keys)">
+                            <ng-container *ngIf="channel.value['accessMode'] === 'RO'">
+                                <ng-container
+                                    *ngIf="channel.value.type == 'BOOLEAN' && channel.key !== '_PropertyEnabled'">


What if the component has another property than 'enabled' that is also boolean?

sfeilmeier · 2020-10-12T07:45:37Z

ui/src/app/edge/live/digitalinput/modal/modal.component.html

+                                        <td style="width:65%">{{ channel.key }}</td>
+                                        <td style="width:36%" class="align_right">
+                                            <ion-icon name="radio-button-off"
+                                                *ngIf="currentData[component.id + '/' + channel.key] === 0; else inputOn">


This is also 'false' if the value is 'undefined', right?

sfeilmeier · 2020-10-12T07:46:38Z

ui/src/app/edge/live/live.component.html

+            <ion-col *ngSwitchCase="'io.openems.edge.io.api.DigitalInput'" size="12" size-lg="6" class="ion-no-padding">
+              <digitalinput></digitalinput>
+            </ion-col>
+          </ng-container>


Why was the indentation for ChannelSingleThreshold changed?

sfeilmeier · 2020-10-12T07:47:17Z

ui/src/app/edge/live/live.module.ts

@@ -63,6 +65,8 @@ import { SymmetricPeakshavingModalComponent } from './peakshaving/symmetric/moda
    SinglethresholdModalComponent,
    StorageModalComponent,
    SymmetricPeakshavingModalComponent,
+    DigitalInputComponent,


Is this required? For the other components it's only the "Modal" component.

sfeilmeier · 2020-10-12T07:48:42Z

ui/src/app/shared/type/widget.ts

@@ -67,6 +68,9 @@ export class Widgets {

        for (let nature of Object.values(WidgetNature).filter(v => typeof v === 'string')) {
            for (let componentId of config.getComponentIdsImplementingNature(nature.toString())) {
+                if (nature === 'io.openems.edge.io.api.DigitalInput' && list.some(e => e.name === 'io.openems.edge.io.api.DigitalInput')) {
+                    continue;
+                }


Why is this specific handling required?

Reviewed-by: Michael Grill <[email protected]> Reviewed-by: Stefan Feilmeier <[email protected]> Co-authored-by: Kai Jeschek <[email protected]> Co-committed-by: Kai Jeschek <[email protected]>

- UI: add notification infrastructure - Battery improvements - Adds `DC_MIN_VOLTAGE` and `DC_MAX_VOLTAGE` to BatteryInverter; implemented in KACO. - Adds ESS-Protection on Generic-ESS - Adds Voltage Regulation in ESS - Modbus/SunSpec: implement BitWords - Refactor SunSpecCodeGenerator - Handle `BITFIELD16` and `BITFIELD32` properly - Allow overriding BITFIELD mapping, e.g. for "Vendor defined events" - ESS Power Solver: improve KEEPING_ALL_EQUAL strategy - Add `OPTIMIZE_BY_KEEPING_ALL_NEAR_EQUAL` strategy; automatically applied as fallback to `OPTIMIZE_BY_KEEPING_ALL_EQUAL`. - Systemd-Networkd: parse [Gateway] in [Route] block - Backend: Send ON_SET_SUM_STATE Event only on change (#1195) - GoodWe: fix sorting of diagnostic bit registers - UI: add warning for systemupdate - Confirmation alert for systemupdate - Standardizing alert for update and EMS restart - UI: adjust translations of alert - Adjust translations of confirmation alert of system update - Adding fix messages for retrofit bnd - Backend b2bwebsocket: fix tests - Gradle builds end with a warning: `Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.` - This is due to this error ``` > Task :io.openems.backend.b2bwebsocket:test No test executed. This behavior has been deprecated. This will fail with an error in Gradle 9.0. There are test sources present but no test was executed. Please check your test configuration. Consult the upgrading giude for further information: https://docs.gradle.org/8.7/userguide/upgrading_version_8.html#test_task_fail_on_no_test_executed ``` - UI: improve systemLog - Adding searchbar to be able to filter systemlog (case-insensitive) - Adding debug level filter - Enable Copying of systemlog - Improved filter for Non condensed output -> splitting debugLog string if line-breaks provided - add toggle to reconfigure [condensedOutput](https://github.com/OpenEMS/openems/blob/develop/io.openems.edge.controller.debug.log/src/io/openems/edge/controller/debuglog/Config.java#L23-L24) - Power Solver: OPTIMIZE_BY_KEEPING_ALL_NEAR_EQUAL - GoodWe: fix impossible high Battery Power values - FENECON Home Battery add stop functionality - STOP Home battery on very low cell voltage. Adds Channels: - LOW_MIN_VOLTAGE_WARNING: Low min voltage warning | Niedriger Ladezustand der Batterie, da die Batterie nicht durch den Wechselrichter beladen werden kann. Ohne Beladung schaltet sich die Batterie demnächst ab, um sich selbst zu schützen - LOW_MIN_VOLTAGE_FAULT: Low min voltage fault | Niedriger Ladezustand. Die Batterie schaltet sich demnächst ab, um sich selbst zu schützen - LOW_MIN_VOLTAGE_FAULT_BATTERY_STOPPED: Low min voltage fault - Battery stopped | Batterie wurde wegen zu niedrigem Ladezustand abgeschaltet. Bitte kontaktieren Sie Ihren Installateur - Backend Alerting: fix NPE, general improvements - make local variables final - remove `public` from interface methods - add null check to `Message.compareTo` - Update Debian settings - Do not install man-pages, locales, docs - Weidmüller IO: fix reading of CurrentModuleList - Fixed hidden ClassCastException in `readCurrentModuleList()` - CI: Improve gradle scripts - UI: fix duplicate legend item generation for same aliases - UI: Extend Eslint - Extend EsLint to make implementing lifecycle interfaces mandatory - UI: Hide Menu in login page - TimeOfUseTariff rabot.charge implementation --------- Co-authored-by: Sebastian Asen <[email protected]> Co-authored-by: Lukas Rieger <[email protected]> Co-authored-by: Stefan Feilmeier <[email protected]> Co-authored-by: Hueseyin Sahutoglu <[email protected]> Co-authored-by: Bishal Ghimire <[email protected]> Co-authored-by: Pooran Chandrashekaraiah <[email protected]> Co-authored-by: Sagar Venu <[email protected]> Co-authored-by: Kai Jeschek <[email protected]> Co-authored-by: Manoj-Kumar Varikela <[email protected]> Co-authored-by: Saeid Nikoubin Boroujeni <[email protected]>

UI: added widget to show digital input states

4221b1b

sfeilmeier requested a review from fabianfnc August 3, 2020 10:07

sfeilmeier assigned fabianfnc Aug 11, 2020

andreashummelsberger and others added 2 commits September 15, 2020 13:18

Digital Input Widget: refactoring, bugfixing, translation

18ff25d

Merge branch 'develop' into feature/ui/digitalinputwidget

d1f70be

UI DigitalInput Widget:

4c1dd02

*underline component alias *disable property channels

add channel.value.type == 'BOOLEAN'

06d74de

fabianfnc approved these changes Oct 2, 2020

View reviewed changes

fabianfnc merged commit 577623f into OpenEMS:develop Oct 2, 2020

sfeilmeier reviewed Oct 12, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI: added widget to show digital input states #1195

UI: added widget to show digital input states #1195

ahummelsberger commented Jul 31, 2020

fabianfnc commented Aug 19, 2020

ahummelsberger commented Aug 20, 2020

fabianfnc commented Aug 21, 2020 •

edited

Loading

ahummelsberger commented Aug 24, 2020

fabianfnc commented Oct 1, 2020 •

edited

Loading

ahummelsberger commented Oct 2, 2020

fabianfnc commented Oct 2, 2020

sfeilmeier commented Oct 2, 2020

fabianfnc left a comment

sfeilmeier left a comment

sfeilmeier Oct 12, 2020

sfeilmeier Oct 12, 2020

sfeilmeier Oct 12, 2020

sfeilmeier Oct 12, 2020

sfeilmeier Oct 12, 2020

sfeilmeier Oct 12, 2020

sfeilmeier Oct 12, 2020

UI: added widget to show digital input states #1195

UI: added widget to show digital input states #1195

Conversation

ahummelsberger commented Jul 31, 2020

fabianfnc commented Aug 19, 2020

ahummelsberger commented Aug 20, 2020

fabianfnc commented Aug 21, 2020 • edited Loading

ahummelsberger commented Aug 24, 2020

fabianfnc commented Oct 1, 2020 • edited Loading

ahummelsberger commented Oct 2, 2020

fabianfnc commented Oct 2, 2020

sfeilmeier commented Oct 2, 2020

fabianfnc left a comment

Choose a reason for hiding this comment

sfeilmeier left a comment

Choose a reason for hiding this comment

sfeilmeier Oct 12, 2020

Choose a reason for hiding this comment

sfeilmeier Oct 12, 2020

Choose a reason for hiding this comment

sfeilmeier Oct 12, 2020

Choose a reason for hiding this comment

sfeilmeier Oct 12, 2020

Choose a reason for hiding this comment

sfeilmeier Oct 12, 2020

Choose a reason for hiding this comment

sfeilmeier Oct 12, 2020

Choose a reason for hiding this comment

sfeilmeier Oct 12, 2020

Choose a reason for hiding this comment

fabianfnc commented Aug 21, 2020 •

edited

Loading

fabianfnc commented Oct 1, 2020 •

edited

Loading