-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add support for advertising-only sensors #360
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #360 +/- ##
==========================================
+ Coverage 51.38% 55.36% +3.97%
==========================================
Files 35 42 +7
Lines 5704 6750 +1046
==========================================
+ Hits 2931 3737 +806
- Misses 2773 3013 +240
|
3a2aad9
to
de77dea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am apparently watching this repo so closely that I even get notified about draft PRs. I gave it a skim and it looks good so far.
btsensor/src/bthome.rs
Outdated
/// | ||
/// In other words, the value stored should be divided by 10 to the power of this number to get | ||
/// the actual value. | ||
fn decimal_point(self) -> i32 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably call this denominator
or something, and make it return an integer or float (1.0 or 100.0 or 1000.0)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
d9d226c
to
6a6f4ad
Compare
It's only needed for decoding.
d2afe32
to
b05902d
Compare
Distinguish between integer and float properties.
This reduces the amount of duplication.
We were skipping 4 bytes rather than 3.
It can be shared with BTHome v2.
They are pretty similar to v1 events, so reuse the same types.
d9b7fa0
to
b9f5389
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔋📏💡⚖️⚡️🏎️🌡️💦🪫🥶🚪🎤🫨🪟🎛️
btsensor/src/bthome/v1.rs
Outdated
} | ||
|
||
/// Attempts to decode the given service data as a BTHome v1 advertisement. | ||
pub fn decode(mut data: &[u8]) -> Result<Vec<Element>, DecodeError> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(optional suggestion) move this onto Element and call it decode_all()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
{ 0x00, PacketId, u8, read_u8, "packet ID", "" }, | ||
{ 0x51, Acceleration, u16, read_u16, "acceleration", "m/s²" }, | ||
{ 0x01, Battery, u8, read_u8, "battery", "%"}, | ||
{ 0x12, Co2, u16, read_u16, "CO2", "ppm"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what order are these in? Is that how they're listed in the spec?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is the order from the spec.
No description provided.