WIP: Layer 2-Implementierung #6

Draft
thomas wants to merge 14 commits from layer2_dev into main
Owner

In diesem Branch wird das Layer2-Protokoll nach der Spezifikation in doc/hamnet70.adoc implementiert.

In diesem Branch wird das Layer2-Protokoll nach der Spezifikation in `doc/hamnet70.adoc` implementiert.
thomas added 4 commits 2024-07-01 21:00:31 +02:00
thomas self-assigned this 2024-07-01 21:00:40 +02:00
thomas changed title from Layer 2-Implementierung to WIP: Layer 2-Implementierung 2024-07-01 21:00:49 +02:00
thomas force-pushed layer2_dev from 0fc63f5f69 to 4bb4017623 2024-07-01 21:04:18 +02:00 Compare
thomas added 1 commit 2024-07-01 21:26:14 +02:00
rudi_s requested changes 2024-07-03 09:01:51 +02:00
Dismissed
rudi_s left a comment
Collaborator
No description provided.
@ -0,0 +51,4 @@
uint16_t *tmp = ham64->addr;
memset(ham64->addr, 0, 4*sizeof(uint16_t));
Collaborator

memset(ham64->addr, 0, sizeof(ham64->addr));

`memset(ham64->addr, 0, sizeof(ham64->addr));`
Author
Owner

Da war ich mir nicht sicher, ob das sizeof(ham64->addr) das richtige tut oder doch die Größe von einem Pointer liefert. Aber müsste schon passen. Danke.

Da war ich mir nicht sicher, ob das `sizeof(ham64->addr)` das richtige tut oder doch die Größe von einem Pointer liefert. Aber müsste schon passen. Danke.
Collaborator

sizeof liefert die Größe des jeweiligen Objekts, das funktioniert soweit ich weiß immer. Ausnahmen sind nur so Dinge wie foo[] am Ende vom struct (da ist dann sizeof == 0).

`sizeof` liefert die Größe des jeweiligen Objekts, das funktioniert soweit ich weiß immer. Ausnahmen sind nur so Dinge wie `foo[]` am Ende vom struct (da ist dann `sizeof == 0`).
rudi_s marked this conversation as resolved
@ -0,0 +137,4 @@
"BROADCAST",
"IPV6_MULTICAST",
"IPV4_MULTICAST",
"RESERVED"};
Collaborator

Minor nit: Put }; on a new line and add a trailing , after "RESERVED"?

Minor nit: Put `};` on a new line and add a trailing `,` after `"RESERVED"`?
rudi_s marked this conversation as resolved
@ -0,0 +173,4 @@
void ham64_format(const ham64_t *ham64, char *out)
{
for(uint8_t i = 0; i < ham64->length; i++) {
sprintf(out + 5 * i, "%04X-", ham64->addr[i]);
Collaborator

Off-by-one overflow (found by fsanitize) if ham64->length == 4 because sprintf NUL-terminates its output but there's not enough space for - and NUL. Possible fix:

 void ham64_format(const ham64_t *ham64, char *out)
 {
        for(uint8_t i = 0; i < ham64->length; i++) {
-               sprintf(out + 5 * i, "%04X-", ham64->addr[i]);
+               const char *fmt = i == 3 ? "%04X" : "%04X-";
+               sprintf(out + 5 * i, fmt, ham64->addr[i]);
        }

        out[5*ham64->length - 1] = '\0';

Off-by-one overflow (found by fsanitize) if `ham64->length == 4` because `sprintf` NUL-terminates its output but there's not enough space for `-` and NUL. Possible fix: ``` void ham64_format(const ham64_t *ham64, char *out) { for(uint8_t i = 0; i < ham64->length; i++) { - sprintf(out + 5 * i, "%04X-", ham64->addr[i]); + const char *fmt = i == 3 ? "%04X" : "%04X-"; + sprintf(out + 5 * i, fmt, ham64->addr[i]); } out[5*ham64->length - 1] = '\0'; ```
rudi_s marked this conversation as resolved
@ -0,0 +45,4 @@
memcpy(header->src_addr.addr, encoded + 2, 2 * header->src_addr.length);
memcpy(header->dst_addr.addr, encoded + 2 + 2 * header->src_addr.length, 2 * header->dst_addr.length);
Collaborator

I think the length of encoded should be passed as well or invalid src/dst lengths can cause an overread.

I think the length of `encoded` should be passed as well or invalid src/dst lengths can cause an overread.
rudi_s marked this conversation as resolved
@ -0,0 +21,4 @@
uint8_t tx_seq_nr; //!< sequence number of this packet
uint8_t rx_seq_nr; //!< sequence number expected next from he other side
Collaborator

Add the maximum value (15) to the comment as the whole uint_8 cannot be used?

Add the maximum value (15) to the comment as the whole `uint_8` cannot be used?
rudi_s marked this conversation as resolved
@ -0,0 +1,50 @@
#include <stdio.h>
Collaborator

I attached test_ham64.c as suggestion to fix the issues I described below.

I attached `test_ham64.c` as suggestion to fix the issues I described below.
rudi_s marked this conversation as resolved
@ -0,0 +7,4 @@
bool test_decode(const char *ref, const ham64_t *ham64)
{
char decoded[13];
decoded[12] = 0;
Collaborator

Only initializing the last byte is problematic because ham64_decode_callsign doesn't NUL-terminate with unsupported inputs, causing unitialized stack data to be printed (and with my patch validated) below.

Only initializing the last byte is problematic because `ham64_decode_callsign` doesn't NUL-terminate with unsupported inputs, causing unitialized stack data to be printed (and with my patch validated) below.
rudi_s marked this conversation as resolved
@ -0,0 +15,4 @@
ham64_format(ham64, ham64_format_buf);
const char *typestr = ham64_addr_type_to_string(ham64_get_addr_type(ham64));
printf("»%s« → [%u] %s (%s) → [%zd] »%s«\n", ref, ham64->length, ham64_format_buf, typestr, decoded_len, decoded);
Collaborator

These printed results should be verified by the test as well.

These printed results should be verified by the test as well.
rudi_s marked this conversation as resolved
@ -0,0 +47,4 @@
test_decode("", &short_1);
test_decode("", &short_last);
test_decode("", &broadcast);
}
Collaborator

The return values are not checked.

The return values are not checked.
rudi_s marked this conversation as resolved
thomas added 2 commits 2024-07-04 20:54:29 +02:00
thomas added 1 commit 2024-07-04 21:12:25 +02:00
thomas force-pushed layer2_dev from ee5c62167a to 153ff61dcd 2024-07-04 21:19:32 +02:00 Compare
thomas added 1 commit 2024-07-04 21:37:46 +02:00
rudi_s approved these changes 2024-07-04 23:21:05 +02:00
rudi_s left a comment
Collaborator

Looks good now :-)

Looks good now :-)
thomas added 1 commit 2024-07-11 23:26:15 +02:00
thomas added 5 commits 2024-07-18 22:59:03 +02:00
thomas added 1 commit 2024-07-18 23:27:48 +02:00
This should enable basic retransmission handling.
thomas added 1 commit 2024-07-19 21:59:51 +02:00
thomas added 1 commit 2024-07-19 22:14:21 +02:00
Before this commit, samples could be dropped during packet reception, which for
sure destroys the current packet and potentially the following one. Now samples
are stored even if the squelch „closes“, and decoding will potentially fail due
to high noise, but not due to missing samples.
thomas added 1 commit 2024-07-19 22:26:38 +02:00
thomas added 1 commit 2024-07-20 00:04:49 +02:00
thomas added 1 commit 2024-07-20 00:27:19 +02:00
thomas added 6 commits 2024-07-20 01:04:25 +02:00
thomas added 5 commits 2024-07-20 22:33:16 +02:00
thomas added 1 commit 2024-07-21 17:23:37 +02:00
thomas added 1 commit 2024-07-22 21:41:23 +02:00
thomas added 1 commit 2024-07-22 22:20:27 +02:00
thomas added 8 commits 2024-07-24 20:33:38 +02:00
thomas added 2 commits 2024-07-27 00:50:34 +02:00
thomas added 1 commit 2024-08-09 22:16:27 +02:00
thomas added 1 commit 2024-08-09 22:18:34 +02:00
thomas added 1 commit 2024-08-09 22:23:37 +02:00
16 is not allowed because it leads to an overflow in sequence numbers, making
them ambiguous.
thomas added 1 commit 2024-08-14 19:14:14 +02:00
thomas force-pushed layer2_dev from 9b7bae5bd8 to ecb8840064 2024-08-16 22:11:28 +02:00 Compare
thomas force-pushed layer2_dev from ecb8840064 to 017eb221f1 2024-08-16 22:14:14 +02:00 Compare
Author
Owner

Statusupdate: da 1:1-Kommunikation jetzt ganz gut funktioniert, haben wir diesen Zwischenstand bereits gemerged. Jetzt geht es an das Protokoll für mehrere Nutzer an einem Digipeater.

Statusupdate: da 1:1-Kommunikation jetzt ganz gut funktioniert, haben wir diesen Zwischenstand bereits gemerged. Jetzt geht es an das Protokoll für mehrere Nutzer an einem Digipeater.
thomas added 1 commit 2024-08-25 20:28:34 +02:00
A template for the config file is provided in src/config.h.template. It must be
copied to src/config.h and adapted for force users to set their own station
call sign.
thomas added 1 commit 2024-08-25 22:28:03 +02:00
The new module is not used yet, but this is a preparation for the future
multi-client networking support.
thomas added 1 commit 2024-08-25 23:27:45 +02:00
thomas added 1 commit 2024-08-27 18:08:25 +02:00
Mermaid is more beautiful, but the tool stack is really annoying. A Chrome
browser should not be necessary to generate some SVGs.
thomas added 1 commit 2024-08-27 21:13:42 +02:00
doc: automatic builds and deployment using Forgejo Actions
All checks were successful
/ build-doc (push) Successful in 14s
/ deploy-doc (push) Successful in 9s
f686635837
Actions run in a custom Docker image built by the scripts in `doc/docker`.
There are two jobs that run in sequence:

- `build-doc`: builds the documentation from the AsciiDoc sources.
- `deploy-doc`: Uploads the generated files to http://0fm.de
thomas added 4 commits 2024-08-27 22:02:25 +02:00
thomas added 1 commit 2024-08-30 15:16:35 +02:00
doc: don't use external resources
All checks were successful
/ build-hamnet70 (push) Successful in 26s
/ build-doc (push) Successful in 15s
/ deploy-doc (push) Has been skipped
3f4a50bdce
Embedding (only) parts of MathJax is a bit ugly ...
thomas added 1 commit 2024-08-30 23:47:11 +02:00
doc: start documenting the Layer 2 packets (WIP)
All checks were successful
/ build-hamnet70 (push) Successful in 26s
/ build-doc (push) Successful in 15s
/ deploy-doc (push) Has been skipped
c61a7a7cf7
thomas added 3 commits 2024-09-12 23:55:19 +02:00
This should help with occasional jumps in the estimated carrier frequency on
receivers running frequency tracking.
rx: reduce squelch threshold from 10 to 6 dB
All checks were successful
/ build-hamnet70 (push) Successful in 27s
/ build-doc (push) Successful in 19s
/ deploy-doc (push) Has been skipped
4dc2c60c8b
During tests it was determined that this is much more reliable. The theory is
that the first packet of a burst was frequently not detected because the
squelch opened too late if the channel was very noisy.

Unfortunately, the squelch now opens very often even if no signal is present.
This will be improved in the future.
All checks were successful
/ build-hamnet70 (push) Successful in 27s
/ build-doc (push) Successful in 19s
/ deploy-doc (push) Has been skipped
This pull request is marked as a work in progress.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin layer2_dev:layer2_dev
git checkout layer2_dev

Merge

Merge the changes and update on Forgejo.
git checkout main
git merge --no-ff layer2_dev
git checkout main
git merge --ff-only layer2_dev
git checkout layer2_dev
git rebase main
git checkout main
git merge --no-ff layer2_dev
git checkout main
git merge --squash layer2_dev
git checkout main
git merge --ff-only layer2_dev
git checkout main
git merge layer2_dev
git push origin main
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: amateurfunk/hamnet70#6
No description provided.