Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Questions and Improvements? #2

Open
lthiery opened this issue Jun 21, 2013 · 6 comments
Open

Questions and Improvements? #2

lthiery opened this issue Jun 21, 2013 · 6 comments

Comments

@lthiery
Copy link

lthiery commented Jun 21, 2013

Hello!

I love the idea of this type of abstraction library and it looks like it has a lot of great things going for it. I'm a little confused at the moment about certain things.

  1. Why the finite list of sensors types?
    Can't we just store sensors in an array and provide some compiler definitions to get to friendly named ones?
    For example:

define LIGHT sensor[0]

And to event.LIGHT() would call it instead

The reason is, what if I have sensors that aren't on this list? What if I have multiples of one sensor? Sure I could easily modify this library to accommodate that but shouldn't an abstraction library not need to be updated per application?

I am sure this has a large part to do with the SIs but that seems more like something that needs to be a convention for people writing the drivers. It can't really be enforced by explicitly naming the sensors.

  1. Can we have a single location with sensor_ids and what sensor has taken it?
    If I am about to create a driver for a new sensor, I want to make sure the sensor_id is not taken! We could just keep track of this in the README perhaps...

Anyway, I'm really glad there's some people thinking about this. Maybe my desires for a library are a little different then what this library intends on having here.

Another thing I am interested in is decoupling getEvent() into: startEvent(), getEventRaw() and getEvent(). This would be useful so that a microcontroller could launch many sensor conversion in a series without waiting to hear back from them. Data could be transmitted in raw form offloading the parsing to SI to another system. getEvent() could still be there for those who just want the parsed form as soon as it can be available.

@lthiery
Copy link
Author

lthiery commented Jun 24, 2013

In trying to write a sensor base class, I have realized a reason why sensor types may be explicitly listed: to allow for more specific data types to be used. The way I'm trying to approach it, without explicitly listing sensor types, I'll be stuck imposing X about of bits on every sensor....

@lthiery
Copy link
Author

lthiery commented Jun 24, 2013

I have written a class that reflects the limitations and capabilities of my approach. I think that what it comes down to is that what I'm looking for is probably a little different than what this project is trying to accomplish.

I want to be able to orchestrate multiple sensors with two objectives:

  1. never needing to change the sensor base class to accommodate new sensor types
  2. being able to orchestrate the timing of sensors easily, between requesting data, reading it in, and parsing it.

That being said, I will try to keep my base class compatible with this project, so that derivatives can work for both.

Here is what I have so far:

#include "Arduino.h"

class Sensor
{
    public:
        Sensor(const uint32_t uuid, const uint32_t dataLength){
            _uuid = uuid;
            _dataLength = dataLength; //length of raw data
        }
        void getSensor(){
            Serial.print("Sensor UUID: ");
            Serial.println(_uuid);
        };
        void getEvent(){
            startEvent();
            getEventRaw();
            parseEvent();       
        };
        uint8_t getSize() { return _dataLength; }
        byte* raw;
    protected:
        virtual void startEvent();
        virtual void getEventRaw();
        virtual void parseEvent();
    private:
        uint32_t _uuid;
        uint8_t _dataLength;
};

#define UUID 0xDEADBEEF
#define LENGTH_OF_DATA 4

class TemperatureSensor: public Sensor
{
    public:
        TemperatureSensor():Sensor(UUID, LENGTH_OF_DATA){
            //give sensor base the pointer to our data array
            raw = &rawData[0]; 
        }
        void startEvent(){}
        void getEventRaw(){
            //just populating with dummy data
            for(int i=0; i<LENGTH_OF_DATA; i++){
                rawData[i]=LENGTH_OF_DATA-i;        
            }
        }
        void parseEvent(){}
        byte rawData[LENGTH_OF_DATA];
};

#endif

@johndavidmiller
Copy link

A sensor abstraction layer makes perfect sense - it's just good software hygiene, rather than forcing apps to deal with arbitrary scalings and such. That said, I don't think it's necessary or desirable to force them all into the same data struct, nor is it necessary, say, to have the version in every packet - 36 bytes is pretty bulky, especially if you have multiple sensors and/or if that data is gonna be sent somewhere else, e.g., to a Pi for further processing.

Which gets to my real request: SPI instead of (or at least as an option) to I2C. Arduino and Pi both have plenty of pins, so finding (N_sensors + 3) available pins should not be a problem.

Sure, a lot of people building these may just be excited to get something - anything - working quickly, then move on, but once you try to really use the stuff, performance gets to be an issue, choosing SPI instead of I2C and keeping the API efficient (while still easy to use) are important.

-- jdm

@johndavidmiller
Copy link

Actually, it looks like the Adafruit 10DOF board does not have SPI, only I2C and some interrupt pins. I'm using the individual boards, of which only L3DG20 has SPI. Sigh.

In any case, the problem I see with the Unified driver is that it uses Euler angles, which will "gimbal lock" - with Euler angles applied sequentially, axes become ambiguous at 90 degree rotations. For example, if you rotate roll 90 degrees, then the yaw axis and roll axis are the same. Try the example with the bunny, and roll 90 - the bunny will flip around crazily in yaw.

The proper thing is to perform rotations using quaternions - 4D vectors, {x,y,z,w}, which unambiguously represent 3D rotation. This is standard practice in 3D graphics (which use a 4x4 matrix to simultaneously encode and accumulate translation, rotation, and scale). You can always report Euler angles for users to better understand, but all the internal math really must be in quaternions, unless you're sure that you'll never approach 90 degree rotations. That seems like an unnecessary limitation, when quaternions are a relatively simple and known solution.

-- jdm

@bitogre
Copy link

bitogre commented May 12, 2015

I agree that combining multiple sensor data types is clunky and, in my case, reduces its usability. It is extra data that is being force upon me to move around. Then there is the issue of covering to Floats and converting to undesired units (m/s^2 vs g-force) causes extra computations that have to be undone and use up valuable clock cycles (even more problematic on something like a Arduino Uno or other 8bit micro-controller).

I am not sure if I should do a complete re-write for embedded application with limits on CPU and memory or add additional methods that allow me to get at the intermediate data before it is converted to the unified units. I am leaning more in the direction of adding additional methods but I am open to suggestions.

zacharyburnett added a commit to UMDBPP/Sensor that referenced this issue Feb 29, 2016
@cdolan92
Copy link

cdolan92 commented Sep 7, 2016

Any plans to support the LIS3DH board in this project? It seems to be pretty popular and low cost. Here is a link to the Library

Edit: It appears it is supported per Adafruit's documentation on the LIS3DH?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants