scanse / sweep-sdk

Sweep SDK
MIT License
90 stars 85 forks source link

Simplify azimuth conversion #90

Closed dcyoung closed 7 years ago

dcyoung commented 7 years ago

Scope of changes

Simplifies the conversion of the azimuth field. See #89 Removes the overly complicated conversion method (u16_to_f32) from the protocol header, and updates the protocol docs.

Known Limitations

Perhaps the docs should also include example conversions in languages other than js... as that really isn't the most likely use case. It was chosen as the example language more for simplicity and readability (almost like pseudo code). But most users will likely be using c++ or something similar.

MikeGitb commented 7 years ago

I think u16_to_f32 is a misnomer (and actually was one of the reasons I got confused initially). As far as I can tell that function is specific to the conversion of the angle from wire representation to float, so why not call it accordingly, like e.g. either

The first one has the advantage that it already produces the result in the right format, so there isn't a double conversion, the latter is the more generic

MikeGitb commented 7 years ago

OK, never mind, I can make that PR myself

dcyoung commented 7 years ago

@MikeGitb Completely agree!