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

Add OS detection and temperature sensor support for Asentria SiteBoss #7655

Merged
merged 6 commits into from Nov 14, 2017

Conversation

Projects
None yet
4 participants
@thecityofguanyu
Contributor

thecityofguanyu commented Nov 6, 2017

…Initial commit. OS definition/polling, temp sensor discovery/polling, MIBs, logo, snmprec test

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

@@ -0,0 +1,12 @@
os: siteboss

This comment has been minimized.

@laf

laf Nov 7, 2017

Member

Is this the OS /firmware name or does it not have one and so the product name is used?

@laf

laf Nov 7, 2017

Member

Is this the OS /firmware name or does it not have one and so the product name is used?

This comment has been minimized.

@thecityofguanyu

thecityofguanyu Nov 7, 2017

Contributor

The latter. Did not have one (that I could find) and so the product name was used.

@thecityofguanyu

thecityofguanyu Nov 7, 2017

Contributor

The latter. Did not have one (that I could find) and so the product name was used.

Show outdated Hide outdated includes/definitions/siteboss.yaml Outdated
Show outdated Hide outdated includes/discovery/sensors/temperature/siteboss.inc.php Outdated
@laf

Some inline comments.

The logo will need to be 32x32 btw unless it's an SVG.

@laf

We're trying to convert everything to yaml so I'd love to get this in as it looks like it should be possible.

Would you be able to provide a walk of the device as per http://docs.librenms.org/Support/FAQ/#faq8 and I'll come up with a patch for you?

Show outdated Hide outdated includes/polling/sensors/temperature/siteboss.inc.php Outdated
@thecityofguanyu

This comment has been minimized.

Show comment
Hide comment
@thecityofguanyu

thecityofguanyu Nov 8, 2017

Contributor

@laf By all means. See paste linked below for SNMPwalk output:

https://p.libren.ms/view/d9f6db7d

The reason I'm unsure of how to make it work as YAML is because of the conditional that I included in the PHP here:

 if ($data[1] == '1') {

In that snippet, $data[1] is value content of an SNMPwalk over esIndexPC. That value has to be equal 1 for it to be a temperature sensor. If you just took all esIndexPC values without the conditional, then you would end up with non-temperature indexes that you don't want to consider.

Contributor

thecityofguanyu commented Nov 8, 2017

@laf By all means. See paste linked below for SNMPwalk output:

https://p.libren.ms/view/d9f6db7d

The reason I'm unsure of how to make it work as YAML is because of the conditional that I included in the PHP here:

 if ($data[1] == '1') {

In that snippet, $data[1] is value content of an SNMPwalk over esIndexPC. That value has to be equal 1 for it to be a temperature sensor. If you just took all esIndexPC values without the conditional, then you would end up with non-temperature indexes that you don't want to consider.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Nov 9, 2017

Member

@laf By all means. See paste linked below for SNMPwalk output:

Needs to be with . rather than an OID as I need the whole thing to simulate the device.

Member

laf commented Nov 9, 2017

@laf By all means. See paste linked below for SNMPwalk output:

Needs to be with . rather than an OID as I need the whole thing to simulate the device.

@thecityofguanyu

This comment has been minimized.

Show comment
Hide comment
@thecityofguanyu
Contributor

thecityofguanyu commented Nov 9, 2017

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Nov 10, 2017

Member

@thecityofguanyu I've pushed to your branch so pull in the changes and test.

@murrant This includes core changes to sensor discovery - thoughts?

Member

laf commented Nov 10, 2017

@thecityofguanyu I've pushed to your branch so pull in the changes and test.

@murrant This includes core changes to sensor discovery - thoughts?

value: esPointValueInt
num_oid: .1.3.6.1.4.1.3052.12.1.1.1.1.6.
descr: esPointName
skip_values:

This comment has been minimized.

@murrant

murrant Nov 10, 2017

Member

Hmm, I thought about having the format like this:
{{ $esIndexPC }} != 1 and parsing it out, but I'm not sure which is better.

@murrant

murrant Nov 10, 2017

Member

Hmm, I thought about having the format like this:
{{ $esIndexPC }} != 1 and parsing it out, but I'm not sure which is better.

This comment has been minimized.

@laf

laf Nov 10, 2017

Member

Yeah I saw your comment in the other PR but that relies on good formatting and a good parser. I chose to make things specific which should lead to less issues with a negligible increase in yaml size.

@laf

laf Nov 10, 2017

Member

Yeah I saw your comment in the other PR but that relies on good formatting and a good parser. I chose to make things specific which should lead to less issues with a negligible increase in yaml size.

@thecityofguanyu

This comment has been minimized.

Show comment
Hide comment
@thecityofguanyu

thecityofguanyu Nov 11, 2017

Contributor

@laf Just pulled down your changes. All tests good for me.

Contributor

thecityofguanyu commented Nov 11, 2017

@laf Just pulled down your changes. All tests good for me.

@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier Nov 13, 2017

The inspection completed: 3 new issues

scrutinizer-notifier commented Nov 13, 2017

The inspection completed: 3 new issues

@laf

laf approved these changes Nov 14, 2017

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Nov 14, 2017

Member

Should be good to go now @librenms/reviewers

Member

laf commented Nov 14, 2017

Should be good to go now @librenms/reviewers

@murrant murrant merged commit 64aed60 into librenms:master Nov 14, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
@thecityofguanyu

This comment has been minimized.

Show comment
Hide comment
@thecityofguanyu

thecityofguanyu Nov 14, 2017

Contributor

@laf

This does not appear to convert F values to celsius after your YAML conversions? I must have not tested the pull closely enough, so my bad there.

From discovery:

Discover sensor: .1.3.6.1.4.1.3052.12.1.1.1.1.6.16.1.1, 16.1.1, siteboss, Sperry Remote, snmp, 1, 1, , 71
SQL[SELECT COUNT(sensor_id) FROM `sensors` WHERE `poller_type`= 'snmp' AND `sensor_class` = 'temperature' AND `device_id` = '283' AND sensor_type = 'siteboss' AND `sensor_index` = '16.1.1'] 
SQL[SELECT * FROM `sensors` WHERE `sensor_class` = 'temperature' AND `device_id` = '283' AND `sensor_type` = 'siteboss' AND `sensor_index` = '16.1.1'] 
.Final sensor value: 0
Final sensor value: 0
Array
(
    [siteboss] => Array
        (
            [16.1.1] => 1
        )

)

From poller:

Modules status: Global+ OS  Device  
#### Load poller module sensors ####
SQL[SELECT `sensor_class` FROM `sensors` WHERE `device_id` = '283' GROUP BY `sensor_class`] 
SQL[SELECT * FROM `sensors` WHERE `sensor_class` = 'temperature' AND `device_id` = '283'] 
SNMP[/usr/bin/snmpget -v1 -c 'public' -OUQnt -M /opt/librenms/mibs:/opt/librenms/mibs/asentria udp:asentria_spe00:161 .1.3.6.1.4.1.3052.12.1.1.1.1.6.16.1.1]
.1.3.6.1.4.1.3052.12.1.1.1.1.6.16.1.1 = 70

Checking (snmp) temperature Sperry Remote... 
70 
RRD[update asentria_spe00/sensor-temperature-siteboss-16.1.1.rrd N:70 --daemon unix:/var/run/rrdcached.sock]
RRDtool Output: OK u:0.00 s:0.00 r:9.13
Sending asentria_spe00.sensor.sensor.temperature.siteboss.16_1_1.sensor 70 1510688002
SQL[UPDATE `sensors` set `sensor_current` ='70',`sensor_prev` ='69',`lastupdate` =NOW() WHERE `sensor_class` = 'Temperature' AND `sensor_id` = '4161'] 
Contributor

thecityofguanyu commented Nov 14, 2017

@laf

This does not appear to convert F values to celsius after your YAML conversions? I must have not tested the pull closely enough, so my bad there.

From discovery:

Discover sensor: .1.3.6.1.4.1.3052.12.1.1.1.1.6.16.1.1, 16.1.1, siteboss, Sperry Remote, snmp, 1, 1, , 71
SQL[SELECT COUNT(sensor_id) FROM `sensors` WHERE `poller_type`= 'snmp' AND `sensor_class` = 'temperature' AND `device_id` = '283' AND sensor_type = 'siteboss' AND `sensor_index` = '16.1.1'] 
SQL[SELECT * FROM `sensors` WHERE `sensor_class` = 'temperature' AND `device_id` = '283' AND `sensor_type` = 'siteboss' AND `sensor_index` = '16.1.1'] 
.Final sensor value: 0
Final sensor value: 0
Array
(
    [siteboss] => Array
        (
            [16.1.1] => 1
        )

)

From poller:

Modules status: Global+ OS  Device  
#### Load poller module sensors ####
SQL[SELECT `sensor_class` FROM `sensors` WHERE `device_id` = '283' GROUP BY `sensor_class`] 
SQL[SELECT * FROM `sensors` WHERE `sensor_class` = 'temperature' AND `device_id` = '283'] 
SNMP[/usr/bin/snmpget -v1 -c 'public' -OUQnt -M /opt/librenms/mibs:/opt/librenms/mibs/asentria udp:asentria_spe00:161 .1.3.6.1.4.1.3052.12.1.1.1.1.6.16.1.1]
.1.3.6.1.4.1.3052.12.1.1.1.1.6.16.1.1 = 70

Checking (snmp) temperature Sperry Remote... 
70 
RRD[update asentria_spe00/sensor-temperature-siteboss-16.1.1.rrd N:70 --daemon unix:/var/run/rrdcached.sock]
RRDtool Output: OK u:0.00 s:0.00 r:9.13
Sending asentria_spe00.sensor.sensor.temperature.siteboss.16_1_1.sensor 70 1510688002
SQL[UPDATE `sensors` set `sensor_current` ='70',`sensor_prev` ='69',`lastupdate` =NOW() WHERE `sensor_class` = 'Temperature' AND `sensor_id` = '4161'] 
@lock

This comment has been minimized.

Show comment
Hide comment
@lock

lock bot May 16, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

lock bot commented May 16, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

@lock lock bot locked as resolved and limited conversation to collaborators May 16, 2018

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