EcmaTC53 / spec

Ecma TC53 spec work
23 stars 11 forks source link

Compound Sensor Discussion #19

Open dtex opened 2 years ago

dtex commented 2 years ago

I wanted to bring @rwaldron into this discussion because I know it is something he has dealt with in the past and he may have some insight that will be valuable.

Rick, since ECMA-419 is now a real live spec the committee is moving on to some new things to complement it. One of those things is a collection of hardware component class specifications. Obviously, you've got some experience there and a question that came up that seems very similar to some discussions we've had in the past so I thought it would be smart to bring you in.

I've pasted the relevant bits from our August meeting below.

Excerpt from August Meeting Notes

PH: Looking at the LSM303DLHC Accelerometer & Magnetometer sensor. This is actually a compound sensor. It incorporates 2+ sensors in one package. This one has an accelerometer and a magnetometer.

PH: We went around on how to handle this in the past. This one takes a physical package for two components and integrates them into one package. It’s weird but not uncommon.

PH: In this draft, we decided not to have a new module for the part number because we can address each sensor class directly. We did draft a document explaining how to use the part number and how to instantiate, configure, and sample each sensor type. In this case, for example, the I2C address is different in the combined package vs the constituent packages, so we did make a note of that.

PH: Thoughts on if this is the right way to handle this?

PH: Now there’s another compound sensor, the MPU9250, which is implemented in a different way. Here, it looks like one sensor with multiple different parameters - an accelerometer, a gyroscope, and a magnetometer accessible via pass-through communication.

PH: Both the accelerometer and gyroscope are returned in one sample object.

PH: What I don’t like about this is that X, Y, Z is repeated twice with X, gyroX, etc. It’s messy. We could do sample.accelerometer.x, sample.gyroscope.x, and so on to avoid naming conflicts.

AC: In the Moddable SDK, we configure whether you want the accelerometer or gyroscope coming back. We don’t support getting both at the same time. But if you want to get both, making them into sub-objects seems correct.

DB: Agreed.

PH: In other cases, sensors like the AHT10 can get into messy naming. We need to access humidity.humidity and temperature.temperature, for example, to avoid naming conflicts.

DB: A sensor actually reports an ADC value; would we instead have humidity.value?

PH: Yes, we could, though the current property name is humidity because conversion is done within the class itself.

AC: And value versus raw may not extend to other sensor types, e.g. x_raw vs. x_value.

Questions for Rick

When a compound sensor wraps two components that are addressed directly is there value in wrapping them in a single class for the device?

When reading a sample from a compound sensor should the data be flat or should each component be its own property of the sample?

Property Description
x A number representing the acceleration along the X-axis in m/s²
y A number representing the acceleration along the Y-axis in m/s²
z A number representing the acceleration along the Z-axis in m/s²
gyroX A number representing the rotation along the X-axis in º/s
gyroY A number representing the rotation along the Y-axis in º/s
gyroZ A number representing the rotation along the Z-axis in º/s

vs.

Property Description
`accelerometer An object containing the accelerometer's sample data
accelerometer.x A number representing the acceleration along the X-axis in m/s²
accelerometer.y A number representing the acceleration along the Y-axis in m/s²
accelerometer.z A number representing the acceleration along the Z-axis in m/s²
`gyroscope An object containing the gyroscope's sample data
gyroscope.x A number representing the rotation along the X-axis in º/s
gyroscope.y A number representing the rotation along the Y-axis in º/s
gyroscope.z A number representing the rotation along the Z-axis in º/s

Johnny-Five does the latter.

And what about the humidity.humidity situation? @phoddie can you remind me why humidity needs to be an object. It's not clear to me how there might be a naming conflict. A quick note, a humidity sensor is a hygrometer so it should probably be hygrometer.humidity or hygrometer.relativeHumidity

phoddie commented 2 years ago

@dtex, thanks for opening this issue so we can work through this.

In the first edition of Ecma-419 we set aside the topic of compound sensors. The excellent recent PR from @mkellner brought that to light. He implemented Ecma-419 style sensor classes for several compound sensors. His approach was to merge al the sensor values into the root of the returned sample record. For some compound sensors, such as Temperature and Humidity, the result looks straightforward.

{
   "temperature": 5,
   "humidity": 11
}

For others -- like one that combines Gyroscope and Accelerometer -- the result is imperfect because they both contain x, y, and z properties. The solution @mkellner used was to change the name of one:

{
   "x": 0.2,
   "y": 0.2,
   "z": 0.2,
   "gyroX": 0.2,
   "gyroY": 0.2,
   "gyroZ": 0.2
}

I don't think that's ideal. I'd like the properties of a gyroscope sample, for example, to always have the same name, regardless of whether it is a standalone gyroscope or a gyroscope that is part of a compound sensor. My preference is to do what you explain Johnny-Five does:

{
   "accelerometer": {
       "x": 0.2,
       "y": 0.2,
       "z": 0.2

   },
   "gyroscope": {
       "x": 0.2,
       "y": 0.2,
       "z": 0.2
    }
}

Patrick Soquet noted that we could maintain a flat sample structure if the property names themselves were less generic. For example, you might use xAcceleration instead of x for an accelerometer. That would work, but it would require coordination across all sensor classes and all vendor specific sensor properties. That seems impractical. Also, first edition has been published using generic names already, so it would be an incompatible change (that might be acceptable at this stage, but we probably shouldn't jump too quickly at breaking changes).

The approach of having separate objects for the sample of each compound sensor (what Johnny-Five does and what I prefer) is redundant in the sample case, as shown by the Humidity and Temperature compound sensor. The samples object for Temperature and Humidity are just:

{
   "temperature": 5
}
{
   "humidity": 11
}

Merging those as objects in the same style as the Accelerator and Gyroscope combination gives:

{
    "temperature": {
       "temperature": 5
    },
    "humidity": {
       "humidity": 11
    }
}

The redundancy is a little awkward. However, it allows very clear, consistent organization of the sample object returned by compound sensors. I like that it is always explicit which properties are part of each sample (e.g. in the case of an accelerometer + gyroscope, the object structure tells you which properties are part of the accelerometer reading and which are part of the gyroscope reading). This is "obvious" so far with the normative properties but wouldn't necessarily be with vendor additions.

And what about the humidity.humidity situation? @phoddie can you remind me why humidity needs to be an object. It's not clear to me how there might be a naming conflict.

The naming conflict can pop-up when non-normative properties are added to the sample by vendors.

A quick note, a humidity sensor is a hygrometer so it should probably be hygrometer.humidity or hygrometer.relativeHumidity

I think we'd need to change first edition to do that. The text does explain that it is relative humidity. Perhaps It is enough to update the text to clarify that its s a hygrometer?

rwaldron commented 2 years ago

PH: What I don’t like about this is that X, Y, Z is repeated twice with X, gyroX, etc. It’s messy. We could do sample.accelerometer.x, sample.gyroscope.x, and so on to avoid naming conflicts.

Yes, this is absolutely the best way to solve this!

When reading a sample from a compound sensor should the data be flat or should each component be its own property of the sample?

The second table is much better than the first table (we do it the second way in J5).

As for awkward situations like humidity.humidity (and so on) the pattern I've found that works the best in terms of addressing the broadest cases is to always use the "meter" name of a sensor as the property name for that sensor's data and then it can have whatever properties it needs within that property's object value (barring Gyroscope, because Gyrometer is a device to measure "gyri" in the brain).

{ 
  accelerometer: {
    x, 
    y, 
    z
  },
  barometer: {
    pressure
  }, 
  gyroscope: {
    x, 
    y,
    z
  },
  hygrometer: {
    relativeHumidity
  }, 
  lightmeter (or lumenmeter): {
    incident, 
    reflective
  }, 
  magnetometer: {
    x, 
    y,
    z
  }, 
  thermometer: {
    temperature
  }, 
}
phoddie commented 2 years ago

@rwaldron - thank you for the helpful feedback. Glad to see we are in agreement about how to organize the readings for compound sensors.

The approach to naming you propose is nice.

From a specification perspective, each Sensor Class (Temperature, Ambient light, Accelerometer, Atmospheric pressure, etc,) just needs to specify the property name to use when including the sample in a compound sensor sample. We have implicitly assumed those would use the name of the sensor class in lower case letters. Your insight is that it can be something else entirely. Developers using the API will discover those names easily by viewing the returned sample object in a debugger or console trace.

FWIW - we have a few naming constraints to live with because Ecma-419 1st edition has been approved (!!). But, those appear to be minor -- for example relativeHumidity above will be humidity.

phoddie commented 2 years ago

@dtex - what do you think? I can try to draft some spec text if this direction looks reasonable.

dtex commented 2 years ago

I agree with Rick on both points. I have a few other things that are nagging me, but I realize they are encoded in the approved text of the spec. In hindsight, I wish this section had been non-normative since it can never be a complete, definitive reference. The universe of devices is vast, diverse, and always evolving.

rwaldron commented 2 years ago

But, those appear to be minor -- for example relativeHumidity above will be humidity.

Is that humidity property actually reporting a "relative humidity"?

phoddie commented 2 years ago

Yes. Here's the text:

A number that represents the sampled relative humidity as a percentage

rwaldron commented 2 years ago

Sorry, that was lazy of me :|

(And thank you for the link and for not calling me out for being lazy)

phoddie commented 2 years ago

There's a very rough draft of the spec text for this committed.

This is just a starting point. Please note errors or suggest improvements.

As an aside, the text defines an embedding name for every Sensor Class. That is complete (good) but maybe silly for things like Touch that, perhaps, are never actually in a compound sensor. Maybe it is a good practice to always define these, even if unlikely to be used, as it could avoid incompatible ad-hoc additions by implementors down the road.