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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wip] add things4u lora format #23

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aparcar
Copy link

@aparcar aparcar commented Feb 17, 2019

This is more like a POC or an idea on how it could be implemented. I think this approach is far easier than using protobuf, at least for the scenarios I'm looking into.

Please be aware I never touched any cpp before, highly likely it's the worst possible style 馃檮

Signed-off-by: Paul Spooren [email protected]

Signed-off-by: Paul Spooren <[email protected]>
@aparcar aparcar mentioned this pull request Feb 17, 2019
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.03%) to 98.252% when pulling 5e702af on aparcar:master into 7937133 on thesolarnomad:master.

@joscha
Copy link
Member

joscha commented Feb 18, 2019

I am not sure about this. Before this change, the library is just very opinionated about how things are encoded and decoded. The format is static all the way, which makes it very simple and also very small. Proto is on the other side, where it allows for very dynamic changes at the expense of a few bytes that define the notion of "fields" and allows for depreciation, etc. The change in this pull request is a mixture of both, but not backwards compatible and only forward compatible with extending the library each time (because new opcodes will need to be introduced). Extending the library won't scale, because some people need X, some Y and some Z, which means at the end, the library has opcode X, opcode Y and opcode Z. WDYT?

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.

None yet

3 participants