JeetShetty / GreenPiThumb

https://mtlynch.io/greenpithumb/
Apache License 2.0
82 stars 12 forks source link

Update camera poller to only take pictures in sufficient light. #139

Closed JeetShetty closed 7 years ago

JeetShetty commented 7 years ago

Until now I wasn't really thinking about how the camera poller has access to the queue but doesn't use it. Seems bad? Should I try to find a way to refactor the camera poller code?

118

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.9%) to 80.968% when pulling 8869efddaffcb8b191a6e85a5d11df22e572e77b on camera_light_sensor into 46e6d56c5b447344d3cb051f2a3e769ab25c25f5 on master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.7%) to 80.833% when pulling 92d2343c81529481a7729d80e94f7117bfc7539f on camera_light_sensor into 46e6d56c5b447344d3cb051f2a3e769ab25c25f5 on master.

mtlynch commented 7 years ago

Until now I wasn't really thinking about how the camera poller has access to the queue but doesn't use it. Seems bad? Should I try to find a way to refactor the camera poller code?

Oh yeah, it's a little messy, but I can't think of a way to fix it without adding more complexity. The way that first comes to mind is to create a deeper class hierarchy like

class _SensorPollWorkerBase(object):
  # Base for all poll workers

class RecordProducingPollWorkerBase(_SensorPollWorkerBase):
  # Base class for workers that produce recorsd

class TemperatureWorker(RecordProducingPollWorkerBase):
  ...

But I feel like that's more confusing than what we have now.

Any ideas for different approaches? I think as-is it's fine, but maybe there's a clean way to fix it.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.8%) to 80.865% when pulling 257910843bb0117d977f3005f09cdc9821ef6d05 on camera_light_sensor into 46e6d56c5b447344d3cb051f2a3e769ab25c25f5 on master.

JeetShetty commented 7 years ago

Any ideas for different approaches? I think as-is it's fine, but maybe there's a clean way to fix it.

Yeah nothing good comes to mind right now, but I'll keep thinking about it. In the most recent commit I changed the camera_poller_factory instantiation to use None instead of the actual record queue, but I'm not sure if this is a good idea.