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

Hddtemp fix issue 4 #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

martivo
Copy link

@martivo martivo commented Feb 12, 2019

By closing and opening new connection to hddtemp daemon, we fetch new data with every call. Also the reset of buffer is needed to have where to write new data.

Should fix issue #4

@martivo martivo mentioned this pull request Feb 12, 2019
@Redsandro
Copy link

I'm not sure why a connection to the hddtemp daemon won't give updating values. But this seems to fix things as far as Prometheus is concerned. I hope @ncabatoff will take a look. He must have observed the same problem.

@martivo
Copy link
Author

martivo commented Feb 13, 2019

Every time the data is asked the code executes this

tempsString, err := h.readTempsFromConn()

This in turn uses the Init - where the acutal HTTP happens

conn, err := net.Dial("tcp", h.address)

This was done in the old code only at first run ( https://github.com/ncabatoff/sensor-exporter/pull/5/files#diff-5c2840fe6d87fdb78e1df0d92576984dL164 )

The code uses the same object, so the Init was before only called once/first time. Now it is called every time. I only added the closing of connection(to not leave dangling stuff https://github.com/ncabatoff/sensor-exporter/pull/5/files#diff-5c2840fe6d87fdb78e1df0d92576984dR172) and to reset the buffer ( https://github.com/ncabatoff/sensor-exporter/pull/5/files#diff-5c2840fe6d87fdb78e1df0d92576984dR167 ) .

@Redsandro
Copy link

LGTM. I will add this manually on my server until this is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants