Update camera and telemetry board sketches to those in use on PAN006 #291
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #291 +/- ##
========================================
Coverage 83.81% 83.81%
========================================
Files 51 51
Lines 3367 3367
Branches 422 422
========================================
Hits 2822 2822
Misses 404 404
Partials 141 141Continue to review full report at Codecov.
|
| const int RESET_PIN = 12; | ||
|
|
||
| // Utitlity Methods |
| if (!ready_) { | ||
| Serial.println("Accelerometer not ready, or not present."); | ||
| } else { | ||
| // Check Accelerometer range |
There was a problem hiding this comment.
I think this was commented out in old code but we should assess if we want it or not. I would be in favor of just removing. But since it only runs the first time (i.e. when not ready) then it might be useful.
There was a problem hiding this comment.
Let's leave it in for the moment so that it is in the history, and so that we can figure out "what next".
| if (ready()) { | ||
| accelerometer_.getEvent(&event_); | ||
| orientation_ = accelerometer_.getOrientation(); // Orientation | ||
| has_reading_ = true; |
There was a problem hiding this comment.
Can this get stale in any way? Perhaps an error on getEvent()?
There was a problem hiding this comment.
getEvent() doesn't provide a status output, so we can't tell if something went wrong.
Do you mean can has_reading_ be wrong?
There was a problem hiding this comment.
Yes, wondering if getEvent() could somehow fail and then has_reading would be outdated. Since report() just checks for has_reading it would just keep reporting the same values and we wouldn't be the wiser. Would be nice to reset has_reading to false each time somehow. Might not be necessary though.
| temperature_ = dht_.readTemperature(); | ||
| } | ||
|
|
||
| void report() { |
There was a problem hiding this comment.
Accelerometer returns a bool so we should make consistent.
There was a problem hiding this comment.
How about the other way: remove the bool return from AccelerometerHandler::report() as the value returned isn't currently used.
There was a problem hiding this comment.
Yes, sounds good. Just going for consistency so if not used let's definitely remove.
| void collect() { | ||
| // Force readHumidity to actually talk to the device; | ||
| // otherwise will read at most every 2 seconds, which | ||
| // is sometimes just a too far apart. |
| led_handler.update(); | ||
| Serial.print(report_num); | ||
| led_handler.update(); | ||
| Serial.print(", \"inputs\":"); |
There was a problem hiding this comment.
inputs was not at all clear to me until reading below. Maybe name the key inputs_recvd or something similar.
| turn_pin_off(pin_num); | ||
| } | ||
| break; | ||
| case LED_BUILTIN: |
There was a problem hiding this comment.
We might be able to remove this since the led_handler will just clobber it anyway.
| out the values of the sensors and the settings of the relays. | ||
|
|
||
| This program has a class for each type of sensor, with a common API: | ||
| 1) An Init() method that is called from setup. |
There was a problem hiding this comment.
Camera board has these methods as lower case names. We can make it conform in a PR that moves things to a shared library if you want.
There was a problem hiding this comment.
Good point. I'll clearly be forced to resolve that when extracting the shared classes.
|
|
||
| int led_value = LOW; | ||
| // Utitlity Methods |
| } else if (pin_status == 9) { | ||
| toggle_pin(pin_num); | ||
| } | ||
| // Sort the devices so we get a consistent order from run to run... |
There was a problem hiding this comment.
I'm surprised we need to do this. Is it due to DallasTemperature or the underlying OneWire?
There was a problem hiding this comment.
I don't now recall if there is an inconsistency in the order from run-to-run, but I assume so since I went to the trouble. Fortunately, I did buy some more of those sensors and can test sometime Real Soon Now ®️. The main point is that it doesn't make a lot of sense to have 3 temps listed, but not have any idea which is which.
These have the benefit that they produce their reports faster because they first gather the data needed before they start writing. This reduces the risk that POCS will timeout while reading a single report.
jamessynge
force-pushed
the
fast-output-sketches
branch
from
1352e38 to
b2cfdc8
Compare
These have the benefit that they produce their reports faster because they first gather the data needed before they start writing. This reduces the risk that POCS will timeout while reading a single report.
In addition, the parsing of relay changes in the telemetry sketch is improved so that it can't accidentally shutdown everything.
This includes PR #290, and should be reviewed after that is submitted.