search

Openvario / sensord

Daemon to poll air data from Openvario sensorboard
6 stars 11 forks source link

Adds ds18b20 suport, fixes some codacy scope issues #31

Closed hpmax closed 3 years ago

hpmax commented 3 years ago

If you give me some more explanation as to how to clear the recursive buffer issue and strlen, I'll work on those. usleep->nanosleep will take me longer to get confidence in it. ds18b20 support has been tested by blaubart.

hpmax commented 3 years ago

Looks like some of my scope reduction stuff broke things. I'll work on it and try to implement usleep->nanosleep. Pleas let me know about the other Codacy failures (strlen and the recursive buffer checks).

hpmax commented 3 years ago

Removed usleep, replaced with nanosleep. I do not plan on fixing any other Codacy issues unless someone can help me with it.

kedder commented 3 years ago

Sorry, I have no clue what you are referring to with "recursive buffer issue and strlen".

hpmax commented 3 years ago

Codacy is now giving two types of errors: 1) "Does not handle strings that are not \0-terminated; if given one it may perform an over-read (it could cause a crash if unprotected) (CWE-126). "

strlen() doesn't work properly if a string isn't 0 terminated. In this case, the strings SHOULD be 0 terminated, but it's always somehow conceivable they won't be. In theory this could potentially be fixed by setting the last character to zero, but I am passed a pointer to a character array, so within the scope of the function that is calling strlen, I don't know the length of character array.

2) "Check buffer boundaries if used in a loop including recursive loops (CWE-120, CWE-20). "

This is being given on an I2C read, but I think I also got it on reading with getch(), I don't understand the error at all.

linuxianer99 commented 3 years ago

Puhhh ... That is changing a lot .. Should we split up the changes a little bit ? This is very difficult to review and to track the changes ...

@kedder : What do you think ?

hpmax commented 3 years ago

I agree it's a lot of changes, but they are kind of related. I'm not sure how to separate them out in an effective way. I also worry that I could break things if I try to disentangle them and it would require a lot of effort. I'd like to emphasize that I haven't extensively tested the new features I added, but @blaubart did test several of the sensors on his setup. What I don't want is for my stuff to get too out of sync with the stock sensord variant.

linuxianer99 commented 3 years ago

@hpmax : So is this fixing #32 ??

hpmax commented 3 years ago

@linuxianer99 Yes, this wraps up all of the temperature/humidity sensor support. I have no new stuff planned, although again, I haven't tested it with many of the sensors. If it doesn't work, I think it's unlikely it will break anything you just won't get temperature or humidity readings... and I'm convinced I can get it fixed quickly. All of these sensors work pretty similarly. You send an I2C command to read data, and then perform a read, and then do a CRC check (and they all seem to have the same polynomial), and you often have to do a linear transform (i.e. read out x, and then the result is mx+b).

I will make a couple other points that aren't directly related to this release:

1) Meta-openvario needs to be altered to add compdata to the release. 2) Based on my communications with Elmar (@eku60) I'm less comfortable with the whole compensation scheme that I came up with. However, at this point, I've ceased caring. The next gen board should hopefully fix this issue and give us good results. 3) I'd like to reiterate that there the way sensord, pulseaudio, and variod are started up has a substantial impact on timing jitter problems. All three of these applications are adversely impacted by bad timing. Sensord will cause noise in the pressure sensor outputs. My enhancements to variod did fix click/pop noise when switching between modes (up, down, and mute) by implementing soft start./stop -- but fundamentally if you have an audio buffer underflow with audio on, even if you have an integer number of cycles and the last sample is '0', you still wind up with popping and both timing jitter on pulseaudio AND variod can cause this.

kedder commented 3 years ago

compdata is going to be added in https://github.com/Openvario/meta-openvario/pull/124

Blaubart commented 2 years ago

Can someone do the second check, so it will be included generally to sensord, please? I flow with this sensord 2021 and in my opinion it works great!

linuxianer99 commented 2 years ago

@Blaubart : This is already merged ....

Blaubart commented 2 years ago

OK, but there is the following comment: @linuxianer99 linuxianer99 merged commit 76bf76b into Openvario:master on 27 Feb 1 of 2 checks passed

There is one check missed, no? I compiled last week with sensord, not sensord-testing, and my sensor dosen't work. After compiling with sensord-testing, it works.

linuxianer99 commented 2 years ago

Not working sensors are not a thing, codacy will find. It's more about format and quality of the code.

i am wondering, it is not working, as Uwe reported all is working fine ?! I have no hardware here at the moment, so i have to rely on the Uwe.

Which sensor is not working ?? Maybe, you make a new issue ??

Blaubart commented 2 years ago

sorry, I was awkward in expressing myself. The sensors for humidity and temperature integrated in the new sensor did not work. Everything else went. So I was sure that these changes had not yet been implemented in the "normal" sensord.