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

variable length ecdsa signature [GV2-193] #1306

Merged
merged 6 commits into from
Mar 17, 2023
Merged

variable length ecdsa signature [GV2-193] #1306

merged 6 commits into from
Mar 17, 2023

Conversation

notoriaga
Copy link
Contributor

Description

@swift-nav/devinfra

Makes the signature on MSG_ECDSA_SIGNATURE variable length

API compatibility

This breaks the existing message but it is currently unused

JIRA Reference

https://swift-nav.atlassian.net/browse/GV2-193

Copy link
Contributor

@silverjam silverjam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for changing my mind on this... but every time we try to sneak one of these in we end up regretting it... so maybe we shouldn't try again. One risk is we have already published tooling that will break when it sees the new version of the message -- no new sbp2json has been published for example, but a new rtcm32json has (which now supports decoding SBP) see here: https://github.com/swift-nav/gnss-converters-private/commit/edb13b9e173e933a59d1dcdbe8045b5acaa6e406 (and this was cut after the libsbp submodule was bumped).

@notoriaga
Copy link
Contributor Author

Sorry for changing my mind on this... but every time we try to sneak one of these in we end up regretting it... so maybe we shouldn't try again. One risk is we have already published tooling that will break when it sees the new version of the message -- no new sbp2json has been published for example, but a new rtcm32json has (which now supports decoding SBP) see here: swift-nav/gnss-converters-private@edb13b9 (and this was cut after the libsbp submodule was bumped).

No worries, I switched to a new message

spec/yaml/swiftnav/sbp/signing.yaml Outdated Show resolved Hide resolved
@@ -0,0 +1,1389 @@
/*
* Copyright (C) 2015-2021 Swift Navigation Inc.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we might be missing the new test files for C and Java that cover the new message ID.

@notoriaga notoriaga merged commit 5b11980 into master Mar 17, 2023
@notoriaga notoriaga deleted the steve/signing branch March 17, 2023 00:10
Blast545 pushed a commit that referenced this pull request Apr 26, 2023
* Add signature length field to MsgEcdsaSignature

Co-authored-by: Jason Mobarak <[email protected]>
lkloh pushed a commit that referenced this pull request Apr 27, 2023
* update MSG_ACKNOWLEDGE on demand fields (#1303)

* variable length ecdsa signature [GV2-193] (#1306)

* Add signature length field to MsgEcdsaSignature

Co-authored-by: Jason Mobarak <[email protected]>

* fix signature in MsgCertificateChain [GV2-193]  (#1307)

Co-authored-by: Jason Mobarak <[email protected]>

* benchmark: run on consistent machine type

Benchmarks seem to oscilate between two values, so attempt to compensate
for this by pinning the machine type that things are run on.

* python: add framer msg type filter (#1321)

* python: add msg type filter

To help with speed issues around Python object construction, add an easy
means of avoiding work in the Python API.

* Update python/sbp/client/framer.py

Co-authored-by: Patrick Crumley <[email protected]>

* docker: use buster, stretch is archived

* python: add unit test for framer filter

* python: add unit test for framer message filter

---------

Co-authored-by: Patrick Crumley <[email protected]>

---------

Co-authored-by: Steven Meyer <[email protected]>
Co-authored-by: Jason Mobarak <[email protected]>
Co-authored-by: Patrick Crumley <[email protected]>
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.

4 participants