Add MTW sentence parser #77
Conversation
src/sentences/mtw.rs
Outdated
| let (i, temperature_value) = opt(float)(i)?; | ||
| let (i, unit) = opt(preceded(char(','), one_of("C")))(i)?; | ||
| let unit = unit.map(|ch| match ch { | ||
| 'C' => MtwUnit::Celcius, |
There was a problem hiding this comment.
Are there any other options apart from Celsius? I didn't find anything else, so maybe it's a good idea to just remove the field from the MtwData. The documentation can be specific for the field - only Celsius.
There was a problem hiding this comment.
I've had searched for this but it's kinda strange, it's described as a field in the documentation (and implemented as is in other libs) but this field seems to can only be celsius. Also didn't find any example with something else, so i assume we can remove it.
src/sentences/mtw.rs
Outdated
|
|
||
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub enum MtwUnit { | ||
| Celcius, |
There was a problem hiding this comment.
There's actually a small typo, should be Celsius with an s
src/sentences/mtw.rs
Outdated
| pub temperature: Option<f32>, | ||
| pub temperature_unit: Option<MtwUnit>, |
There was a problem hiding this comment.
- Let's just leave
temperatureand add a documentation block///that it is only "degrees Celsius" - use
double(f64) instead. We've discussed this before and we should unify the api to always usef64.
src/sentences/mtw.rs
Outdated
| assert_eq!(s.checksum, 0x20); | ||
| let mtw_data = parse_mtw(s).unwrap(); | ||
| assert_eq!(Some(17.9), mtw_data.temperature); | ||
| assert_eq!(None, mtw_data.temperature_unit); |
There was a problem hiding this comment.
This test should cause an error instead of Option::None
#54