Skip to content
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 uint64 support for the session id #29

Merged
merged 12 commits into from
Feb 19, 2020

Conversation

DerkJanSpeelman
Copy link
Contributor

@DerkJanSpeelman DerkJanSpeelman commented Feb 8, 2020

Add uint64 support for the missing session id

I wanted to fetch the m_sessionUID inside the PacketHeader (sent inside every message). I found out there wasn't uint64 support with TypeScript 3.1.0 and the included binary-parser (version 1.3.2). So, I upgraded these packages. And everything seems to be working fine.

I'm also running on a newer version of Node (Latest Current Version 13.8.0). Thereby, it added a few version changes to the package-lock.json. But it should run fine from version 12 onwards:

The methods readBigInt64BE, readBigInt64BE, readBigInt64BE, readBigInt64BE are not avilable in your version of nodejs: v10.19.0, you must use v12 or greater

I added m_sessionUID to the PacketHeader

In src\parsers\packets\PacketHeaderParser.ts:18, I added the sessionUID. We might want to change this uint64 to a uint64le

class is extending a namespace

I also found a bug the F1TelemetryClient class. This class extends the EventEmitter namespace, not the class. Just a small fix.

PacketSessionData

I added the m_formula in the PacketSessionData interface. The PacketSessionData with the PacketHeader included now looks like this:

PacketSessionData

Note: check the m_sessionUID and formula attributes. And m_pitSpeedLimit is just the end of the screenshot, the regular data is still there ofcourse.

@DerkJanSpeelman DerkJanSpeelman changed the title Add biguint64 support Add uint64 support for the session id Feb 8, 2020
@jonybur
Copy link
Collaborator

jonybur commented Feb 9, 2020

Hi @DerkJanSpeelman thank you for your PR. This looks good! Can you please check out the tests (update them if necessary) so I can merge your changes? Thank you

@DerkJanSpeelman
Copy link
Contributor Author

Hi @DerkJanSpeelman thank you for your PR. This looks good! Can you please check out the tests (update them if necessary) so I can merge your changes? Thank you

I'd love to, but the Travis CI is setup with Node v10.19.0. Therefore:

The methods readBigInt64BE, readBigInt64BE, readBigInt64BE, readBigInt64BE are not avilable in your version of nodejs: v10.19.0, you must use v12 or greater

@jonybur jonybur merged commit db57bc9 into racehub-io:master Feb 19, 2020
@jonybur
Copy link
Collaborator

jonybur commented Feb 19, 2020

@DerkJanSpeelman merged 👍

jonybur added a commit that referenced this pull request Feb 19, 2020
jonybur added a commit that referenced this pull request Feb 19, 2020
@jonybur
Copy link
Collaborator

jonybur commented Feb 19, 2020

@DerkJanSpeelman I had to revert this PR due because Travis running node 8, 10 wasn't the (only) problem: the tests are failing to check the value of m_sessionUID. The mocks folder possess a parsed version of all the packages, which do not include a value of m_sessionUID. Given that the value m_sessionUID is a bigint, we need to set the tsconfig to target ESNext and make the mocks in .ts format (instead of .json). After that is done then this will be ready for merging 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants