CSSALTlab / Open_Source_Ventilator

Open source for use worldwide and contributions from others worldwide
228 stars 67 forks source link

Version 109 from Dr. Gibby #43

Closed MarshallRawson closed 4 years ago

MarshallRawson commented 4 years ago

Just curious about #ifndef BEENHERE vs VENT_H . Is this for instructional purposes or am I missing something?

So unfortunately, we only have one working prototype and it is been allocated to validate hardware by some very non-programmers(Gordon Gibby and friends) sprinting for the FDA approval. (they emailed me this code). The BEENHERE I believe is just a manifestation of their lack of understanding of how the C-Pre-Proccessor works. I (an experienced embedded developer) was just added to the project recently and and will be receiving a unit as soon as they can solder on together. Then I can go and fix all the things(may require an entire re-write of being honest). To make the code professional grade.

MarshallRawson commented 4 years ago
1. A quibble: line 104 if(current_phase ==2 || current_phase==3), the reasons for magic numbers 2 and 3 are not immediately obvious. Usually current_phase is compared to better defined limits.

2. If I've done the math correctly, the 25% divider and the single 11.5V limit means that if the 12V supply changes by 20 mV the alarm will nuisance trip on and off - I don't know the quality of the supply voltage but some hysteresis may be in order.

Thank you for your comments (keep them coming please)! This is why Open Source is beautiful. I will pass them along to the appropriate boomers.