[feature] driver/netio: use netio via json-api #1234

ActionHOSchT · 2023-07-11T13:39:50Z

Description

Following the contribution-guidelines an additional functionality using the netio-json-api was added to simplify netio handling. To get all actions working powerprotocol.py:NetworkPowerDriver probably needs a refactor.

Tested manually with a Netio-4-Device and in productive-env used with netio-8.

import logging
import time

import pytest

FEATURE = "power"


@pytest.fixture()
def pdu(target):
    return target.get_driver('NetworkPowerDriver')


@pytest.mark.lg_feature(FEATURE)
def test_something(pdu):
    pdu.on()
    # Note that the pdu might not have switched on when requesting it's
    # status immediately
    time.sleep(5)
    assert pdu.get()['State'] == 1
    logging.info("could turn on")

    pdu.cycle()
    time.sleep(5)
    assert pdu.get()['State'] == 1
    logging.info("could cycle")

    pdu.off()
    time.sleep(5)
    assert pdu.get()['State'] == 0
    logging.info("could turn off")

Signed-off-by: Raffael Krakau [email protected]

Checklist

Documentation for the feature

The arguments and description in doc/configuration.rst have been updated

PR has been tested

Following the contribution-guidelines an additional functionality using the netio-json-api was added to simplify netio handling. To get all actions working powerprotocol.py:NetworkPowerDriver probably needs a refactor. Signed-off-by: Raffael Krakau <[email protected]>

Apply suggestions from code review add 'netio_json' section to configuration.rst Signed-off-by: Raffael Krakau <[email protected]> Co-authored-by: Christian Happ <[email protected]>

Bastian-Krause · 2023-07-31T15:19:08Z

labgrid already supports these netio APIs:

netio
Controls a NETIO 4-Port PDU via a simple HTTP API.

netio_kshell
Controls a NETIO 4C PDU via a Telnet interface.

See: https://labgrid.readthedocs.io/en/latest/configuration.html#networkpowerport

Why do you need to implement an additional API?

ActionHOSchT · 2023-08-01T11:17:38Z

labgrid already supports these netio APIs:

netio
Controls a NETIO 4-Port PDU via a simple HTTP API.
netio_kshell
Controls a NETIO 4C PDU via a Telnet interface.

See: https://labgrid.readthedocs.io/en/latest/configuration.html#networkpowerport

Why do you need to implement an additional API?

We had issues using kshell with the 8-port version of netio. (even changed devices)

When we contacted netio they told us, that we must use e.g. the json-api to control the 8-port-netios. Which we did and implemented this netio-json-driver for labgrid.

Considering the manufacturer telling us to use json-api, this is a mandatory feature for people who want to use 8-port netios. Even if the current netio-docs say, that kshell should be possible with the netio PowerPDU 8QS, we use the json-api to control the netios.

[bugfix] add global-var PORT Bug was missed due to old version Signed-off-by: Raffael Krakau <[email protected]>

Emantor · 2023-09-06T10:06:40Z

labgrid/driver/power/netio_json.py

+PORT = 80
+
+
+class __NetioControl:


Why the double underscore?

Initially dunderscore was added to keep NetioControl private and to prevent it from showing up in the doc.(imo a user/dev does only need this info if he already takes a look in the source-code)

With dunderscore the class will be mangled and show up as power__NetioControl giving a better idea what this exactly is.

Can this be resolved?

Would a single underscore still be prefered?

Emantor · 2023-09-06T10:07:43Z

labgrid/driver/power/netio_json.py

+    """
+    Netio-Class to switch sockets via HTTP-JSON-API
+    """
+    __host: str


__host and the variables below are defined as class variables, however they are not used anywhere? Every place uses the instance variables instead.

__host is a private copy of host (added to prevent side-effects)
At line 105 this copy will be used.
__host will be mangled and show up as NetioControl__host which prevents any other operation from accidentally modifying it.

Can this be resolved?

Any further suggestion on how to handle this preferably?

Emantor · 2023-09-06T10:09:06Z

labgrid/driver/power/netio_json.py

+        - 3: Short ON delay,
+        - 4: Toggle (invert the state)
+    """
+    netio = __NetioControl(host, port)


Is the class indirection necessary? How are username and password set?

The class indirection isn't strictly necessary, sure it would also be possible to implement this classless.

The json-api supports/expects username and password, even if they're empty.
afaik NetworkPowerDriver doesn't support to set username and password because NetworkPowerPort doesn't support it.
We/I couldn't find a way to set username and password at exporter-config.yaml.
~~Because of this another PR is on it's way (hopefully PR-ready within 2 weeks).~~

Username and password are prefabricated.
If you want this to be removed I will remove username and password.
If it's ok for you I would keep it for possible "future-use".

No, the class is not necessary, we can adjust that. Just as note, we use the NetioControl class in other projects as well, we just added the Labgrid API functions power_set and power_get to reuse that code and get it working with Labgrid.

How should we continue on that ?

Emantor · 2023-09-06T10:10:11Z

Are the netio devices supported by pdudaemon? In that case it may be easier to use PDUDaemon.

ActionHOSchT · 2023-09-08T08:13:25Z

Are the netio devices supported by pdudaemon? In that case it may be easier to use PDUDaemon.

I don't see the benefits from using PDUDaemon ~~since we look forward to provide the NetworkPowerDriverWithAuth to finally(no offense) set credentials at export-config.yaml.~~

If you or someone else sees the benefit from using PDUDaemon I would be really thankful to get to know them.

ep1cman · 2023-12-05T16:06:26Z

labgrid/driver/power/netio_json.py

+
+            for outputState in responseDict['Outputs']:
+                if outputState['ID'] == socketID:
+                    state = outputState


This should be
state = outputState['State']
Otherwise it always shows as on

set state to ...'State' as mentioned in labgrid-project#1234 (comment) Signed-off-by: Raffael Krakau <[email protected]>

jluebbe · 2023-12-15T10:36:08Z

Are the netio devices supported by pdudaemon? In that case it may be easier to use PDUDaemon.

I don't see the benefits from using PDUDaemon since we look forward to provide the NetworkPowerDriverWithAuth to finally(no offense) set credentials at export-config.yaml.

Why do you want distribute the credentials via the coordinator in the clear to all clients? PDUDaemon current has no authentication either, but it's a HTTP API, so it should be relatively easy to switch to authenticated HTTPS using a reverse proxy. At that point, every user could have personal credentials (would need small changes to the PDUDaemonDriver).

If you or someone else sees the benefit from using PDUDaemon I would be really thankful to get to know them.

With PDUDaemon, we can:

reuse the driver implementations across projects (labgrid, lava and others)
avoid concurrent access from clients to the same PDU by serializing them (some have bugs with parallel access)
"hide" the actual PDUs behind a web service that can be protected via firewalls and/or authentication

ActionHOSchT · 2024-04-04T07:50:44Z

Since I will be no longer collaborating with JUMO and therefore have no setup to test changes, I close this PR.

Bastian-Krause · 2024-04-16T12:20:44Z

Reopened as requested by @JUMO-GmbH-Co-KG.

ActionHOSchT and others added 2 commits July 11, 2023 15:25

Update configuration.rst

b8ed99f

Apply suggestions from code review add 'netio_json' section to configuration.rst Signed-off-by: Raffael Krakau <[email protected]> Co-authored-by: Christian Happ <[email protected]>

Update netio_json.py

c63336b

[bugfix] add global-var PORT Bug was missed due to old version Signed-off-by: Raffael Krakau <[email protected]>

ActionHOSchT force-pushed the feature/driver/netio branch from 78b1d7b to c63336b Compare August 21, 2023 06:35

Emantor requested changes Sep 6, 2023

View reviewed changes

Emantor added needs author info Requires more information from the PR/Issue author enhancement labels Sep 6, 2023

Bastian-Krause assigned Emantor Oct 4, 2023

ep1cman reviewed Dec 5, 2023

View reviewed changes

driver/netio: [bugfix] apply suggestion

d3b48d0

set state to ...'State' as mentioned in labgrid-project#1234 (comment) Signed-off-by: Raffael Krakau <[email protected]>

Emantor assigned ActionHOSchT and unassigned Emantor Mar 25, 2024

ActionHOSchT closed this Apr 4, 2024

Bastian-Krause reopened this Apr 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feature] driver/netio: use netio via json-api #1234

[feature] driver/netio: use netio via json-api #1234

ActionHOSchT commented Jul 11, 2023 •

edited

Loading

Bastian-Krause commented Jul 31, 2023 •

edited

Loading

ActionHOSchT commented Aug 1, 2023

Emantor Sep 6, 2023

ActionHOSchT Sep 8, 2023 •

edited

Loading

ActionHOSchT Feb 14, 2024

SeminSey May 28, 2024

Emantor Sep 6, 2023

ActionHOSchT Sep 8, 2023

ActionHOSchT Feb 14, 2024

SeminSey May 28, 2024

Emantor Sep 6, 2023

ActionHOSchT Sep 8, 2023 •

edited

Loading

ActionHOSchT Feb 14, 2024

ChHapp May 28, 2024

Emantor commented Sep 6, 2023

ActionHOSchT commented Sep 8, 2023 •

edited

Loading

ep1cman Dec 5, 2023

ActionHOSchT Dec 15, 2023

jluebbe commented Dec 15, 2023

ActionHOSchT commented Apr 4, 2024

Bastian-Krause commented Apr 16, 2024

[feature] driver/netio: use netio via json-api #1234

Are you sure you want to change the base?

[feature] driver/netio: use netio via json-api #1234

Conversation

ActionHOSchT commented Jul 11, 2023 • edited Loading

Bastian-Krause commented Jul 31, 2023 • edited Loading

ActionHOSchT commented Aug 1, 2023

Choose a reason for hiding this comment

ActionHOSchT Sep 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ActionHOSchT Sep 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Emantor commented Sep 6, 2023

ActionHOSchT commented Sep 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jluebbe commented Dec 15, 2023

ActionHOSchT commented Apr 4, 2024

Bastian-Krause commented Apr 16, 2024

ActionHOSchT commented Jul 11, 2023 •

edited

Loading

Bastian-Krause commented Jul 31, 2023 •

edited

Loading

ActionHOSchT Sep 8, 2023 •

edited

Loading

ActionHOSchT Sep 8, 2023 •

edited

Loading

ActionHOSchT commented Sep 8, 2023 •

edited

Loading