RSDK-5770 - implement GetReadings API for all sensor types #126
Conversation
gvaradarajan
force-pushed
the
RSDK-5770-readings-api
branch
from
ae5b6ec to
73d2f0c
Compare
micro-rdk-macros/src/lib.rs
Outdated
| let micro_rdk_crate_name = if crate_name == "micro-rdk" { | ||
| "crate" | ||
| } else { | ||
| "micro_rdk" |
There was a problem hiding this comment.
Does this work even if the micro-rdk crate has been pulled in under a different name, per https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#renaming-dependencies-in-cargotoml.
There was a problem hiding this comment.
Thanks for checking. I was curious and I do wonder if we are missing something about how to stably name types from a "peer" crate within a proc macro. On the other hand, I don't think we need to address it right now: I think renaming dependencies is a pretty edge-case sort of thing. Maybe a comment is warranted for now, and should it ever cause problems in the wild we can revisit it.
There was a problem hiding this comment.
i think the solution here rust-lang/rust#54363 (comment) might work even if the crate is renamed
There was a problem hiding this comment.
if this doesn't work then we need to keep a ticket to track this. I agree it's not major but it can be a pain point in the future
| let gen = quote! { | ||
| impl #impl_generics DoCommand for #name #ty_generics #where_clause {} | ||
| impl #impl_generics #crate_ident::common::generic::DoCommand for #name #ty_generics #where_clause {} |
src/common/sensor.rs
Outdated
| @@ -108,3 +126,19 @@ impl Status for FakeSensor { | |||
| })) | |||
| } | |||
| } | |||
|
|
|||
| impl From<f64> for Value { | |||
There was a problem hiding this comment.
These seem more generic than simply being Sensor related. Is there a better home for them?
There was a problem hiding this comment.
I actually meant to remove these, sorry. I don't think it's saving that much effort
@npmenard I initially did this. However, I had the thought that the convenience function is useful to someone implementing a movement sensor or power sensor module who wanted to take the readings returned by the standard implementation and simply add to it in their |
gvaradarajan
force-pushed
the
RSDK-5770-readings-api
branch
from
71bc07f to
6f86530
Compare
also includes improvement for DoCommand macro such that only the macro needs to be imported when someone needs to use it (you don't need to also import the DoCommand trait)