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

peek() function allways return first byte of data #85

Closed
patricklaf opened this issue Jun 27, 2024 · 0 comments · Fixed by #86
Closed

peek() function allways return first byte of data #85

patricklaf opened this issue Jun 27, 2024 · 0 comments · Fixed by #86
Assignees
Labels
bug 🐛 Something isn't working
Milestone

Comments

@patricklaf
Copy link
Contributor

Description
The peek() functions of the EthernetUDP and EthernetClient classes always returns the first byte of a packet.
A fix is proposed.

To Reproduce

Code used:

#include "Arduino.h"

#include <STM32Ethernet.h>

EthernetUDP udp;

void setup() {
    Serial.begin(115200);
    Ethernet.begin(IPAddress(192, 168, 2, 2));
    udp.begin(161);
}

void loop() {
    if (udp.parsePacket()) {
        uint32_t byte = 0;
        while (udp.available()) {
            uint8_t peek = udp.peek();
            uint8_t read = udp.read();
            Serial.printf("Byte %02d peek 0x%02X read 0x%02X\n", byte++, peek, read);
        }
    }
}

Command to generate a packet:

snmpget -v 2c -c public 192.168.2.2 SNMPv2-MIB::sysName.0 -r0 -d

Sending 43 bytes to UDP: [192.168.2.2]:161->[0.0.0.0]:46928
0000: 30 29 02 01  01 04 06 70  75 62 6C 69  63 A0 1C 02    0).....public...
0016: 04 71 FB CD  11 02 01 00  02 01 00 30  0E 30 0C 06    .q.........0.0..
0032: 08 2B 06 01  02 01 01 05  00 05 00                    .+.........

Serial output:

Byte 00 peek 0x30 read 0x30
Byte 01 peek 0x30 read 0x29
Byte 02 peek 0x30 read 0x02
Byte 03 peek 0x30 read 0x01
Byte 04 peek 0x30 read 0x01
Byte 05 peek 0x30 read 0x04
Byte 06 peek 0x30 read 0x06
Byte 07 peek 0x30 read 0x70
Byte 08 peek 0x30 read 0x75
Byte 09 peek 0x30 read 0x62
Byte 10 peek 0x30 read 0x6C
Byte 11 peek 0x30 read 0x69
Byte 12 peek 0x30 read 0x63
Byte 13 peek 0x30 read 0xA0
Byte 14 peek 0x30 read 0x1C
Byte 15 peek 0x30 read 0x02
Byte 16 peek 0x30 read 0x04
Byte 17 peek 0x30 read 0x71
Byte 18 peek 0x30 read 0xFB
Byte 19 peek 0x30 read 0xCD
Byte 20 peek 0x30 read 0x11
Byte 21 peek 0x30 read 0x02
Byte 22 peek 0x30 read 0x01
Byte 23 peek 0x30 read 0x00
Byte 24 peek 0x30 read 0x02
Byte 25 peek 0x30 read 0x01
Byte 26 peek 0x30 read 0x00
Byte 27 peek 0x30 read 0x30
Byte 28 peek 0x30 read 0x0E
Byte 29 peek 0x30 read 0x30
Byte 30 peek 0x30 read 0x0C
Byte 31 peek 0x30 read 0x06
Byte 32 peek 0x30 read 0x08
Byte 33 peek 0x30 read 0x2B
Byte 34 peek 0x30 read 0x06
Byte 35 peek 0x30 read 0x01
Byte 36 peek 0x30 read 0x02
Byte 37 peek 0x30 read 0x01
Byte 38 peek 0x30 read 0x01
Byte 39 peek 0x30 read 0x05
Byte 40 peek 0x30 read 0x00
Byte 41 peek 0x30 read 0x05
Byte 42 peek 0x30 read 0x00

peek() function always returns the first byte of the packet.

Expected behavior
The peek()should return the same data byte as the next read() call.

Explanation

This is the implementation of the UDP peek function:

int EthernetUDP::peek()
{
  uint8_t b;

  // Unlike recv, peek doesn't check to see if there's any data available, so we must.
  // If the user hasn't called parsePacket yet then return nothing otherwise they
  // may get the UDP header
  if (!_remaining) {
    return -1;
  }
  b = pbuf_get_at(_udp.data.p, 0);
  return b;
}

It is just a wrapper arround the lwip function pbuf_get_at(). This function returns the byte at index 0 of the current buffer.
It should returns the byte at the current read position of the current buffer.

Fix

The function is modified to take into account the read position.

int EthernetUDP::peek()
{
  uint8_t b;

  // Unlike recv, peek doesn't check to see if there's any data available, so we must.
  // If the user hasn't called parsePacket yet then return nothing otherwise they
  // may get the UDP header
  if (!_remaining) {
    return -1;
  }
  b = pbuf_get_at(_udp.data.p, _udp.data.p->tot_len - _udp.data.available);
  return b;
}

Serial output:

Byte 00 peek 0x30 read 0x30
Byte 01 peek 0x29 read 0x29
Byte 02 peek 0x02 read 0x02
Byte 03 peek 0x01 read 0x01
Byte 04 peek 0x01 read 0x01
Byte 05 peek 0x04 read 0x04
Byte 06 peek 0x06 read 0x06
Byte 07 peek 0x70 read 0x70
Byte 08 peek 0x75 read 0x75
Byte 09 peek 0x62 read 0x62
Byte 10 peek 0x6C read 0x6C
Byte 11 peek 0x69 read 0x69
Byte 12 peek 0x63 read 0x63
Byte 13 peek 0xA0 read 0xA0
Byte 14 peek 0x1C read 0x1C
Byte 15 peek 0x02 read 0x02
Byte 16 peek 0x04 read 0x04
Byte 17 peek 0x3E read 0x3E
Byte 18 peek 0x12 read 0x12
Byte 19 peek 0x88 read 0x88
Byte 20 peek 0x90 read 0x90
Byte 21 peek 0x02 read 0x02
Byte 22 peek 0x01 read 0x01
Byte 23 peek 0x00 read 0x00
Byte 24 peek 0x02 read 0x02
Byte 25 peek 0x01 read 0x01
Byte 26 peek 0x00 read 0x00
Byte 27 peek 0x30 read 0x30
Byte 28 peek 0x0E read 0x0E
Byte 29 peek 0x30 read 0x30
Byte 30 peek 0x0C read 0x0C
Byte 31 peek 0x06 read 0x06
Byte 32 peek 0x08 read 0x08
Byte 33 peek 0x2B read 0x2B
Byte 34 peek 0x06 read 0x06
Byte 35 peek 0x01 read 0x01
Byte 36 peek 0x02 read 0x02
Byte 37 peek 0x01 read 0x01
Byte 38 peek 0x01 read 0x01
Byte 39 peek 0x05 read 0x05
Byte 40 peek 0x00 read 0x00
Byte 41 peek 0x05 read 0x05
Byte 42 peek 0x00 read 0x00

Now peek() function returns the byte of the packet at the current read position as expected.

Same correction can be applied for the EthernetClient class (untested):

int EthernetClient::peek()
{
  uint8_t b;
  // Unlike recv, peek doesn't check to see if there's any data available, so we must
  if (!available()) {
    return -1;
  }
  b = pbuf_get_at(_tcp_client->data.p, _tcp_client->data.p->tot_len - _tcp_client->data.available);
  return b;
}

Desktop

  • OS: Windows
  • Eclipse CDT version: 2024-06
  • Sloeber version: 4.4.3
  • STM32 core version: 2.7.1
  • Upload method: SWD

Hardware

  • Board Name: Nucleo F767ZI
  • Hardware Revision: Rev B
@patricklaf patricklaf added the bug label Jun 27, 2024
@fpistm fpistm added this to the 1.4.1/1.5.0 milestone Sep 10, 2024
@fpistm fpistm self-assigned this Sep 10, 2024
@fpistm fpistm added bug 🐛 Something isn't working and removed bug labels Sep 11, 2024
@fpistm fpistm linked a pull request Sep 11, 2024 that will close this issue
@fpistm fpistm closed this as completed Sep 11, 2024
@github-project-automation github-project-automation bot moved this from To do to Done in STM32duino libraries Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working
Projects
Development

Successfully merging a pull request may close this issue.

2 participants