-
Notifications
You must be signed in to change notification settings - Fork 6
Fix exponent notation #153
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
base: main
Are you sure you want to change the base?
Conversation
b1c931d
to
ceb1c7e
Compare
Debating using html vs unicode. I think for this one in particular unicode may be the best way to go about it, otherwise we can use |
@@ -127,7 +127,7 @@ This field is optional. In order to indicate that this field is available, the b | |||
> **Note** | |||
> | |||
> The full timestamp information can thus be retrieved using the formula: | |||
> Timestamp(s) = [`Seconds`] + [`Microseconds`] * 32 * 10-6 | |||
> Timestamp(s) = [`Seconds`] + [`Microseconds`] * 32 * 10⁻⁶ |
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.
> Timestamp(s) = [`Seconds`] + [`Microseconds`] * 32 * 10⁻⁶ | |
> Timestamp(s) = [`Seconds`] + [`Microseconds`] * 32e-6 |
I feel we should just write it in plain text scientific notation as in practice that is what people will have to write in code, e.g.: https://github.com/bonsai-rx/harp/blob/main/src/Bonsai.Harp/HarpMessage.cs#L231
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 think this is meant to be read as text not code, so I would go with that IEEE seems to do in their papers and adopt the exponent notation. If you want to go with code-compatible code, then I would suggest upper casing the E as it seems to be more common?
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.
Both upper-case or lower-case works for me. The reason I prefer lower-case is I feel it makes the separation clearer because e
is always smaller in height than any digit and is also faster to type since you don't need to use a keyboard modifier.
The reading notation would be fine, just feels slightly stranger to me since we are combining it with verbatim code elements. In fact looking at the formula again I would reformat it to remove the (s)
since Timestamp(s)
written like this seems like a function definition where s
is the argument.
The other thing that feels out-of-place on a second look is the use of square brackets to reference fields and registers in text, e.g. [Seconds
]. I would drop all of this and simplify the notation overall, adopting formal cross-reference links like we did in the Device document, so perhaps we need to expand the scope of the PR and decide on and fix the notation everywhere.
Assuming we do this, I think we could use either of the following options:
Timestamp = Seconds + Microseconds * 32e-6
or to make it similar to scientific and technical papers we could use a math environment (render below):
$$
Timestamp = Seconds + Microseconds \times 32 \times 10^{-6}
$$
My issue with the math notation is that we should really use \times
rather than *
, but I feel it is clunky to use twice the \times
symbol. Perhaps there is a better way to do it but I couldn't immediately see it.
This PR fixes a small typo in the Binary Protocol specification.
Closes #59