sqozz
/
sem6000
2
2
Fork 3
Code Issues Pull Requests Releases Wiki Activity

Labels Milestones

Collectd Plugin #13

Merged
sqozz merged 3 commits from cfr34k/sem6000:collectd into master 4 years ago
Conversation 1 Commits 3 Files Changed 2
cfr34k commented 4 years ago

This aims to create a plugin for collectd that can read data from multiple SEM6000 devices.

Current progress:

  • Status monitoring works
  • Multiple devices can be read out
  • README updated with some documentation

Not working very well yet:

  • If a configured socket is unreachable, samples are lost for all devices because all devices are queried sequentially and the connect timeout is about 30 seconds ➡️ maybe use one thread per device?
This aims to create a plugin for collectd that can read data from multiple SEM6000 devices. Current progress: - Status monitoring works - Multiple devices can be read out - README updated with some documentation Not working very well yet: - If a configured socket is unreachable, samples are lost for all devices because all devices are queried sequentially and the connect timeout is about 30 seconds :arrow_right: maybe use one thread per device?
sqozz approved these changes 4 years ago
sqozz left a comment
Owner

Just minor nitpicks, definitely happy to have this PR :)
What is your opinion of using the mac to submit values?

Just minor nitpicks, definitely happy to have this PR :) What is your opinion of using the mac to submit values?
README.md
@ -10,0 +25,4 @@
```shell
# mkdir -p /usr/local/lib/collectd/python
# cp collectd/collectd_sem6000.py /usr/local/lib/collectd/python
sqozz commented 4 years ago
Owner

did you want to indicate root rights for these commands (because of the preceding #)? If not, I'd prefer $ as shell indicator :)

did you want to indicate root rights for these commands (because of the preceding `#`)? If not, I'd prefer `$` as shell indicator :)
cfr34k commented 4 years ago
Poster

Yes, the # should indicate required root access.

Yes, the `#` should indicate required root access.
collectd/collectd_sem6000.py Outdated
@ -0,0 +7,4 @@
import collectd
from sem6000 import SEMSocket
import bluepy
sqozz commented 4 years ago
Owner

maybe just import the exception here? I don't know if it makes any performance difference but at least it would be less confusing why bluepy is needed in here.

maybe just import the exception here? I don't know if it makes any performance difference but at least it would be less confusing why bluepy is needed in here.
cfr34k commented 4 years ago
Poster

Sounds reasonable. I'll do that!

Sounds reasonable. I'll do that!
collectd/collectd_sem6000.py
@ -0,0 +97,4 @@
inst['lastsuccess'] = time.time()
val = collectd.Values(plugin = 'sem6000-{}'.format(config['socketname']))
sqozz commented 4 years ago
Owner

I'd prefer to use the BT mac instead of a string as plugin name here. There's a good chance we can read the name of the socket over BT and expose it later with this plugin which would allow a mapping even if the name changes.

I'd prefer to use the BT mac instead of a string as plugin name here. There's a good chance we can read the name of the socket over BT and expose it later with this plugin which would allow a mapping even if the name changes.
cfr34k commented 4 years ago
Poster

Are you sure that the name is actually stored on the socket? My feeling is that that’s only a mapping in the app.

I prefer to have a string in the plugin name, because it indicates the purpose of the measurements (which can change as devices are reused). If you don't like it, just copy the address into SocketName 😉 in the collectd config.

Are you sure that the name is actually stored on the socket? My feeling is that that’s only a mapping in the app. I prefer to have a string in the plugin name, because it indicates the purpose of the measurements (which can change as devices are reused). If you don't like it, just copy the address into `SocketName` :wink: in the collectd config.
cfr34k changed title from WIP: Collectd Plugin to Collectd Plugin 4 years ago
cfr34k commented 4 years ago
Poster

I hereby declare this ready for an initial integration, as I don't have a great solution for the reconnect duration right now.

I hereby declare this ready for an initial integration, as I don't have a great solution for the reconnect duration right now.
sqozz closed this pull request 4 years ago
The pull request has been merged as 353c6f64bb.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b cfr34k-collectd master
git pull collectd

Step 2:

Merge the changes and update on Gitea.
git checkout master
git merge --no-ff cfr34k-collectd
git push origin master
Sign in to join this conversation.
Reviewers
Request review
No reviewers
Labels
Apply labels
Clear labels
No items
No Label
Milestone
Set milestone
Clear milestone
No items
No Milestone
Assignees
Assign users
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: sqozz/sem6000#13
Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'cfr34k/sem6000:collectd'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes