RSDK-4790: Add Readings to TypeScript SDK #191
Conversation
martha-johnston
added
question
martha-johnston
force-pushed
the
rsdk-4790
branch
from
b1b7207 to
3f931e4
Compare
| result[key] = value.toJavaScript(); | ||
| } | ||
| return readings as PowerSensorReadings; | ||
| return result; |
There was a problem hiding this comment.
Why are we using Record<string, unknown> here instead of the PowerSensorReadings type?
There was a problem hiding this comment.
because I thought PowerSensorReadings was a type we were using before there was an API for powersensor.GetReadings
There was a problem hiding this comment.
since sensor did not have that and now sensors and power sensor are using the same api
There was a problem hiding this comment.
We relied on those types in the RC, so now we see these errors:
Error: Type 'unknown' is not assignable to type 'number | undefined'. (ts)
const readings = await powerSensorClient.getReadings();
voltageValue = readings.voltage;
currentValue = readings.current;
/__w/rdk/rdk/web/frontend/src/components/power-sensor/index.svelte:28:5
Error: Type 'unknown' is not assignable to type 'number | undefined'. (ts)
voltageValue = readings.voltage;
currentValue = readings.current;
powerValue = readings.power;
/__w/rdk/rdk/web/frontend/src/components/power-sensor/index.svelte:29:5
Error: Type 'unknown' is not assignable to type 'number | undefined'. (ts)
currentValue = readings.current;
powerValue = readings.power;
} catch (error) {
With these changes we can no longer rely on the SDK to return us current, voltage, or power as a part of the readings response object. Also we no longer know if those fields exist on the response object, and since they are type to unknown we have no idea if they are numbers, strings, objects, etc...
There was a problem hiding this comment.
oh sorry I had no idea, are you seeing this issue with movement sensor as well? or is it still working
There was a problem hiding this comment.
I haven't even tested anything out, the RC is failing it's build checks right now with an SDK version bump. These changes are not passing our standard checks.
add getreadings to movement and power sensor