[GNRC PPP] HDLC definitions by jia200x · Pull Request #5471 · RIOT-OS/RIOT

jia200x · 2016-05-28T16:30:16Z

Hi.

This PR contains the definitions of HDLC header and FCS, required by GNRC PPP.

OlegHahm · 2016-05-31T19:15:41Z

sys/include/net/hdlc/fcs.h

@@ -0,0 +1,88 @@
+/*
+ * Copyright (C) 2015 José Ignacio Alamos <[email protected]>


OlegHahm · 2016-05-31T19:16:36Z

I think you're using tabs instead of white spaces.

jia200x · 2016-10-31T16:34:04Z

@OlegHahm do you think this could be merged? These are generic HDLC definitions but are needed for PPP.

miri64 · 2016-10-31T18:29:00Z

sys/include/net/hdlc/fcs.h

+    0x6b46, 0x7acf, 0x4854, 0x59dd, 0x2d62, 0x3ceb, 0x0e70, 0x1ff9,
+    0xf78f, 0xe606, 0xd49d, 0xc514, 0xb1ab, 0xa022, 0x92b9, 0x8330,
+    0x7bc7, 0x6a4e, 0x58d5, 0x495c, 0x3de3, 0x2c6a, 0x1ef1, 0x0f78
+};


This should be provided externally (i.e. via extern in some C-file), otherwise every file that includes this would need 512b of RAM.

(and we should maybe quickly move this to a non-table solution, since we usually have more ROM than RAM)

((but that we can do after this one is merged))

miri64 · 2016-10-31T18:34:20Z

sys/include/net/hdlc/fcs.h

+ */
+
+/**
+ * @defgroup    net_hdlc_fcs    FCS calculation


Maybe this can be unified with sys_checksum_crc16_ccitt (I know, most likely different polynomial!) somehow?

It's a different polynomial (see this). Maybe add another sys_checksum_crc function?

@jia200x which is it then? According to all sources I can find HDLC uses CCITT (0x1021), too. Anyway, here's a more flexible (with regards to polynomials) and more lightweight implementation of CRC16: #6112

@miri64 PPP uses CRC-CCITT with 0x8408 and 0xffff as initial value. I just read your implementation and I think it can be used for this case.
I will do some tests, in that case I can drop this file.

Cheers

jia200x · 2016-11-02T18:00:25Z

@miri64 are we still on time to merge this? Or should be postponed?

Cheers!

miri64 · 2016-11-03T01:12:12Z

@jia200x the release is already feature freezed since yesterday (as announced multiple times on the devel mailinglist) and the release has branched from master. Even if this PR were marked for the release (which it never was apparently) it would now be difficult to get it into the release, if that is what you want, since there needs to be a backport for it.

If a maintainer finds time between testing and and bugfixing for the release to review this PR, it might get merged into master, but I would not put hopes into this. But we can merge this into master ASAP, so that it will be in 2017.01 most definitely.

OlegHahm · 2017-01-15T15:13:14Z

What are the chances for this release?

jia200x · 2017-01-16T00:23:52Z

Really high. I have been working all this weekend on it. I will try to submit it tomorrow, or tuesday in worst case El 15 ene. 2017 12:13 PM, "Oleg Hahm" <[email protected]> escribió: What are the chances for this release? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5471 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABM8SJOoIjPNAVbeHm5eN-xwzvKCWQ3Rks5rSjeLgaJpZM4IpHBa> .

smlng · 2018-01-15T13:35:03Z

any new here, at least @miri64 comment regarding externalise the table was not addressed yet.

jia200x · 2018-01-15T14:19:06Z

@sming I will drop that file, I think it's better to add that feature in another PR. Also, I think we should stick to #6112 implementation for FCS.

miri64 · 2019-02-18T14:17:05Z

@sming I will drop that file, I think it's better to add that feature in another PR. Also, I think we should stick to #6112 implementation for FCS.

Then let's close this PR for now as memo.

jia200x changed the title ~~HDLC definitions~~ [GNRC PPP] HDLC definitions May 28, 2016

OlegHahm added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking GNRC labels May 31, 2016

OlegHahm reviewed May 31, 2016
View reviewed changes

miri64 assigned OlegHahm May 31, 2016

miri64 reviewed Oct 31, 2016

View reviewed changes

miri64 mentioned this pull request Nov 13, 2016

ucrc16: provide lightweight CRC16 implementation #6112

Merged

jia200x force-pushed the hdlc_hdr branch from 16370e3 to 720afeb Compare January 18, 2017 16:24

miri64 mentioned this pull request Jun 21, 2017

[Link layer] GNRC PPP #5470

Closed

aabadie modified the milestone: Release 2017.07 Jun 23, 2017

aabadie modified the milestones: Release 2017.07, Release 2017.10 Jun 30, 2017

kYc0o mentioned this pull request Oct 30, 2017

Network Stack Low layers #7893

Closed

15 tasks

smlng modified the milestones: Release 2017.10, Release 2018.01 Nov 16, 2017

smlng modified the milestones: Release 2018.01, Release 2018.04 Jan 15, 2018

sys/net/hdlc: Add HDLC headers definition

d173326

jia200x force-pushed the hdlc_hdr branch from da22dbe to d173326 Compare January 15, 2018 14:27

kaspar030 removed this from the Release 2018.04 milestone Apr 16, 2018

miri64 removed the GNRC label Oct 5, 2018

miri64 added the State: archived State: The PR has been archived for possible future re-adaptation label Feb 18, 2019

miri64 closed this Feb 18, 2019

		@@ -0,0 +1,88 @@
		/*
		* Copyright (C) 2015 José Ignacio Alamos <[email protected]>

Conversation

jia200x commented May 28, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OlegHahm commented May 31, 2016

Uh oh!

jia200x commented Oct 31, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jia200x commented Nov 2, 2016

Uh oh!

miri64 commented Nov 3, 2016

Uh oh!

OlegHahm commented Jan 15, 2017

Uh oh!

jia200x commented Jan 16, 2017 via email

Uh oh!

smlng commented Jan 15, 2018

Uh oh!

jia200x commented Jan 15, 2018

Uh oh!

miri64 commented Feb 18, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants