fossasia / pslab-python

Python Library for PSLab Desktop: https://pslab.io
GNU General Public License v3.0
1.62k stars 227 forks source link

Replace firmware implementation of HCSR04 #209

Closed bessman closed 2 years ago

bessman commented 2 years ago

This is basically the same as what I posted earlier with some minor cleanup and added error checking.

bessman commented 2 years ago

One drawback of this implementation is that the timeout isn't configurable. The logic analyzer timeout cannot be shorter than the trigger pulse period (60 ms) times the number of measurements to average. Assuming the sensor itself times out after approximately 24 ms, this means we cannot distinguish a sensor timeout from an object that is actually 4 m away.

Basically, it's up to the user to determine if the result is realistic.

bessman commented 2 years ago

I tried printing out the self._channels list and instead of digital inputs, it's printing digital outputs.

Yeah, I see why now. I was trying to be clever by having the HCSR04 class inherit from LogicAnalyzer and PWMGenerator instead of using instances of them. But the instruments are not designed for cooperative multiple inheritance, so the PWMGenerator._channels attribute overwrote LogicAnalyzer._channels since it's later in the MRO.

I want to fix the inheritance model of pslab-python so that derived instruments and sensors can use cooperative multiple inheritance, but for now I'll fix it by adding instrument instances to the sensor class instead.

I'm surprised that didn't happen earlier when you ran the code snippet in the other PR.

It also causes a timeout for ACK error when lighting up the RGB LED. I had to comment it out to get it run.

This is because 8c0e975 removed the SET_RGB1/2/3 subcommand and replaced them with SET_RGB_COMMON. I commented out that part of sciencelab.py a long time ago and forgot about it. I'll open a PR to fix it.

The snippet you shared in firmware PR works but I changed it a bit to put into a script as follows.

Yep, those changes explain why it's working. They circumvent the inheritance problems outlined above.