search

pimoroni / internet-of-seeds

The code that runs our IKEA VÄXER Internet of Seeds
20 stars 4 forks source link

Move Sensors Into Classes #2

Closed AndiCui closed 6 years ago

AndiCui commented 8 years ago

First step for the ease to use other sensors.

THIS REQUIRES TESTING. I don’t not have a flotilla. Need test to prove it is working.

Name Change? I think those sensor libs should have the same name format. Discussions on how it should be called?

sandyjmacdonald commented 8 years ago

Completely agree. Needs classes for everything, really. Alex Ellis has forked and started on a re-write too. I'll try to cobble it all together into something more sensible when I have a spare afternoon (?!).

Not sure what you mean by name formats? Can you be more specific?

AndiCui commented 8 years ago

by name format i mean the file name;)

another thing to consider:

Sensor Data Handling It should make more sense (at least for me) to generate data for timestamp text in each sensor's class.(because of different systems sense different things, some sense more) But this make it difficult to log and tweet in different formats (RGB, logging separately, joined before tweeting )

sandyjmacdonald commented 8 years ago

The idea behind generating the timestamp once, was that everything - the image, the tweet, the data log - receives exactly the same timestamp. Keeps it simple if you want to tie everything back together again after, i.e. compare images, tables of data, etc.

AndiCui commented 8 years ago

Point taken. It is possible to change the time stamp generation step, so it plots all the values that the sensor have in a consistent order? (Not from a predefined list in the main script of expected values, as some might be empty, and some might not exist in the list)

alexellis commented 8 years ago

Hey @AndiCui in addition to extracting to a new class, it should also be injected in through the constructor meaning any alternative class can plugged in like: a 'sensor' of cpu temp + time.

It's almost like an overlay provider, even people with Flotilla may not have every sensor or every sensor available - so think this is a step in the right direction. Not sure if all sensors need individual timestamps though.

Gadgetoid commented 6 years ago

This PR has been languishing for a while, and it's dangling on the bottom of my TODO list.

Is a rewrite forthcoming, should we be moving to merge this? Can I close this PR for now?