Skip to content

Enexus system output current fix#14324

Merged
murrant merged 12 commits intolibrenms:masterfrom
loopodoopo:ELTEK_fix
Sep 30, 2022
Merged

Enexus system output current fix#14324
murrant merged 12 commits intolibrenms:masterfrom
loopodoopo:ELTEK_fix

Conversation

@loopodoopo
Copy link
Copy Markdown
Contributor

@loopodoopo loopodoopo commented Sep 8, 2022

System output current is not represented correctly, there was a divisor by 10 which is not needed:

Before the change:
image

Usage on the rectifier (load changed a fraction to 73A between screenshots....):
image

After the change:
image

DO NOT DELETE THE UNDERLYING TEXT

Please note

Please read this information carefully. You can run ./lnms dev:check to check your code before submitting.

  • Have you followed our code guidelines?
  • If my Pull Request does some changes/fixes/enhancements in the WebUI, I have inserted a screenshot of it.
  • If my Pull Request makes discovery/polling/yaml changes, I have added/updated test data.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

@loopodoopo loopodoopo marked this pull request as ready for review September 8, 2022 14:58
@PipoCanaja
Copy link
Copy Markdown
Contributor

PipoCanaja commented Sep 8, 2022

All is good, except you need to run save-test-data.php -o enexus -v '' to also upgrade the tests JSON enexus.json. That will make the tests happy and the change will be ready for merge.
Thanx

Copy link
Copy Markdown
Contributor

@PipoCanaja PipoCanaja left a comment

Choose a reason for hiding this comment

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

./scripts/save-test-data.php -o enexus -v '' to run

@Jellyfrog Jellyfrog added the Device 🖥️ New or added device support label Sep 8, 2022
@loopodoopo
Copy link
Copy Markdown
Contributor Author

I think I tried all options including new variant (which I believe is not neccesery).

Please advise.

@PipoCanaja
Copy link
Copy Markdown
Contributor

Hi @loopodoopo
The answer is in the test results :

1) LibreNMS\Tests\OSModulesTest::testOS with data set "enexus_eltek-smartpack2-syst" ('enexus', 'eltek-smartpack2-syst', array('os', 'sensors'))
OS enexus: Discovered sensors data does not match that found in tests/data/enexus_eltek-smartpack2-syst.json

That means that the variant eltek-smartpack2-syst must be "save-test-data.php" again.

Something like ./scripts/save-test-data.php -o enexus -v 'eltek-smartpack2-syst' .

That being said, looking at the folter with the tests :

librenms@librenms:~$ ls -altr tests/data/enexus*
-rw-rw-r-- 1 librenms librenms 74616 Sep  8 14:08 tests/data/enexus.json
-rw-rw-r-- 1 librenms librenms 64042 Sep  8 14:08 tests/data/enexus_smartpacks.json
-rw-rw-r-- 1 librenms librenms 72477 Sep  8 14:08 tests/data/enexus_eltek-smartpack2-syst.json
-rw-rw-r-- 1 librenms librenms 59707 Sep  8 14:08 tests/data/enexus_sp2touch.json
-rw-rw-r-- 1 librenms librenms 63997 Sep  8 14:08 tests/data/enexus_smartpacks2.json 

So you should probably do it on all variants here, to be on the safe side.

@murrant
Copy link
Copy Markdown
Member

murrant commented Sep 9, 2022

@loopodoopo Hmm, can you check if there is a firmware setting that can affect SNMP output?

@loopodoopo
Copy link
Copy Markdown
Contributor Author

@PipoCanaja I learned something. Thank you!

@murrant murrant modified the milestones: 22.9.0, 22.10.0 Sep 19, 2022
@loopodoopo
Copy link
Copy Markdown
Contributor Author

Can we get this change merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Device 🖥️ New or added device support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants