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

[feature] spe_switch: Add spe_switch power feature #1274

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ActionHOSchT
Copy link

This PR adds the feature to set power for a phoenix_fl_switch over telnet.

Devices connected with single-pair-ethernet can now be set to power off/on/cycle.

To verify, this Driver was tested on a FL SWITCH 2303-8SP1 with FW-version 3.27.01 BETA.
Even if it's a BETA, we got the information that the power control won't experience breaking changes.

grafik
grafik

grafik

Checklist

  • Documentation for the feature
  • Tests for the feature
  • The arguments and description in doc/configuration.rst have been updated
  • PR has been tested

Copy link
Member

@Bastian-Krause Bastian-Krause left a comment

Choose a reason for hiding this comment

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

Looks mostly good to me.

CI test errors are not your fault, #1277 fixes them.

FYI: there is also https://github.com/pdudaemon/pdudaemon, a project that focuses only on PDUs. labgrid's PDUDaemonDriver interfaces with it. So it might be worth a thought to implement this rather over there. Let us know how you think about this.

Note: we should wait for #1277 and rebase this to make CI run this before merging.

Comment on lines 198 to 201
``phoenix_fl_switch``
Controls a single-pair-ethernet powerswitch via telnet.
Tested on a FL SWITCH 2303-8SP1 with FW-version 3.27.01 BETA

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure about the backend name. The complete name of your device is "Phoenix Contact FL SWITCH 2303-8SP1". Could you find out which products use the same "telnet API"? Maybe have look at the firmware? Maybe something like "phoenixcontact_fl_2300"?

Choose a reason for hiding this comment

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

Name has been adjusted.

@Bastian-Krause Bastian-Krause added enhancement needs rebase Needs a rebase onto the master branch, maintainter could probably not push to submitter branch. labels Oct 9, 2023
@jluebbe jluebbe assigned jluebbe and unassigned ActionHOSchT Dec 15, 2023
Copy link
Member

@jluebbe jluebbe left a comment

Choose a reason for hiding this comment

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

Please also squashfs the existing commits into one with a title following our usual conventions, like "driver/power: add backend for Phoenix Contact FL 2303-8SP1 SPE switch".

Adding support for the telnet scheme in labgrid/driver/powerdriver.py should be a separate commit.

Otherwise it looks good.

Comment on lines 20 to 21
username = "admin"
password = "private"
Copy link
Member

Choose a reason for hiding this comment

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

Instead of hardcoding the credentials, you could use a URL like telnet://admin:[email protected]/ as the host. You'd need to add telnet as a scheme in https://github.com/labgrid-project/labgrid/blob/master/labgrid/driver/powerdriver.py#L194

Copy link
Author

Choose a reason for hiding this comment

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

This would indeed be nice.

I used netio_kshell.py as inspiration for the phoenix_fl_switch.py .
Implementing a new scheme would be some effort.

If you want me to put this suggestion on our todo this will be noted ?

Copy link

Choose a reason for hiding this comment

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

Is this request mandantory to get this PR upstream ?
The overall goal is to port all labgrid-drivers to PDUDaemon, right ?

Choose a reason for hiding this comment

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

Commits have been squashed as requested.

@jluebbe jluebbe assigned ActionHOSchT and unassigned jluebbe Dec 15, 2023
@ActionHOSchT
Copy link
Author

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

@Bastian-Krause
Copy link
Member

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

Co-authored-by: Jan Lübbe <[email protected]>
Signed-off-by: Semin Buljevic <[email protected]>

Rename phoenix_fl_switch.py to phoenixcontact_fl_2300.py

Signed-off-by: semin.buljevic <[email protected]>
@Emantor
Copy link
Member

Emantor commented May 28, 2024

According to the product page, this device also supports snmp, does it support power switching via SNMP as well? This would be less fragile vs implementing this via pexpect over telnet.

@ChHapp
Copy link

ChHapp commented May 28, 2024

Hi @Emantor,

We contacted Phoenix Contact round about seven months ago and asked them how we could remotely control the ports.
They prepared a test firmware (FW version 3.27.01 BETA) for us and we sent them feedback that everything worked just fine with this beta FW. So they told us that this feature would definitely be part of the next release. I understand your concerns about the Telenet API, to be honest I have no idea if it will break soon or not.
To put it in a nutshell, as far as we know, that's the way to go at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs rebase Needs a rebase onto the master branch, maintainter could probably not push to submitter branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants