mapbox / pbf

A low-level, lightweight protocol buffers implementation in JavaScript.
BSD 3-Clause "New" or "Revised" License
801 stars 107 forks source link

question: Do we support Timestamp type? #94

Closed amitguptagwl closed 6 years ago

amitguptagwl commented 6 years ago

Here is the complete list supported by protobuf 3: https://developers.google.com/protocol-buffers/docs/proto3#json

I face an issue in compiling proto file with Timestamp field. So I was wondering if I'm using the wrong syntax or it is not supported.

message Signals {
    Timestamp start = 2;
    repeated sint32 signal = 1;
}
mourner commented 6 years ago

Yeah, we don't currently support importing google protobuf extensions. Same as #92

As a workaround, you can inline Timestamp definition from Google:

https://github.com/protocolbuffers/protobuf/blob/f8d56e929e35e822f62912fa0c91aa1e640cc373/src/google/protobuf/timestamp.proto#L123-L135

amitguptagwl commented 6 years ago

Great thanks

amitguptagwl commented 6 years ago

@mourner One more question. For float field, If I input 987654321 I get 987654336 back. I know that is because of 'ieee754' encoding. But just want to ensure that if this problem persist with actual google library as well? (TBH I didn't tested that yet)

amitguptagwl commented 6 years ago

@mourner I've created gist extracted from google protobuf implementation. However it doesn't work for any number. So I'm not sure what's wrong I'm doing.

kjvalencik commented 6 years ago

@amitguptagwl float in protobuf corresponds to a 32-bit float. Single precision floats can only represent integers up to 16777216 accurately.

const assert = require('assert');

const num = 987654321;
const expected = 987654336;
const buf = Buffer.alloc(4);

buf.writeFloatLE(num);

const actual = buf.readFloatLE();

assert.strictEqual(actual, expected);

If you use a double precision float (64-bit, double in protobuf), it can accurately represent that number.

amitguptagwl commented 6 years ago

Thanks @kjvalencik . I actually noticed that google protobuf implementation stops user to write data which is out of range or can't read same. Shouldn't we do the same?

kjvalencik commented 6 years ago

I don't think so. Those bounds check could impact performance. Also, there are some intentional inaccuracies. For example, int64 and uint64 both use a JS Number which can't represent the range.

amitguptagwl commented 6 years ago

Hmm