patricklaf / SNMP

Simple Network Management Protocol library for Arduino
https://patricklaf.github.io/SNMP/
MIT License
14 stars 1 forks source link

Compiling issues with arduino-esp32 #9

Closed cydanil closed 5 months ago

cydanil commented 5 months ago

Hi Patrick,

Thank you for taking the time to develop and maintain this library, it's very useful to me. I'm using it with the Arduino framework and an esp32 (arduino-esp32 v. 3.0.1).

I see that in the latest release, 1.4.0, you've added support for some remaining BER types, including TYPE_IPADDRESS.

This kinda breaks compilation as the esp32 IPAddress constructor expects one of ip_addr_t*, char*, uint8_t*, or uint32_t, whereas SequenceBER::decode initializes an IPAddressBER with nullptr, resulting in the following error:

/home/cydanil/Arduino/libraries/SNMP/src/BER.cpp: In member function 'virtual const uint32_t SNMP::SequenceBER::decode(unsigned char*)':
/home/cydanil/Arduino/libraries/SNMP/src/BER.cpp:422:44: error: conversion from 'int' to 'IPAddress' is ambiguous
  422 |                     ber = new IPAddressBER(0);
      |                                            ^
In file included from /home/cydanil/.arduino15/packages/esp32/hardware/esp32/3.0.1/cores/esp32/Arduino.h:196,
                 from /home/cydanil/Arduino/libraries/SNMP/src/BER.h:4,
                 from /home/cydanil/Arduino/libraries/SNMP/src/BER.cpp:1:
/home/cydanil/.arduino15/packages/esp32/hardware/esp32/3.0.1/cores/esp32/IPAddress.h:115:3: note: candidate: 'IPAddress::IPAddress(const ip_addr_t*)'
  115 |   IPAddress(const ip_addr_t *addr);
      |   ^~~~~~~~~
/home/cydanil/.arduino15/packages/esp32/hardware/esp32/3.0.1/cores/esp32/IPAddress.h:72:3: note: candidate: 'IPAddress::IPAddress(const char*)'
   72 |   IPAddress(const char *address);
      |   ^~~~~~~~~
/home/cydanil/.arduino15/packages/esp32/hardware/esp32/3.0.1/cores/esp32/IPAddress.h:69:3: note: candidate: 'IPAddress::IPAddress(const uint8_t*)'
   69 |   IPAddress(const uint8_t *address);
      |   ^~~~~~~~~
/home/cydanil/.arduino15/packages/esp32/hardware/esp32/3.0.1/cores/esp32/IPAddress.h:67:3: note: candidate: 'IPAddress::IPAddress(uint32_t)'
   67 |   IPAddress(uint32_t address);
      |   ^~~~~~~~~
/home/cydanil/Arduino/libraries/SNMP/src/BER.h:435:28: note:   initializing argument 1 of 'SNMP::IPAddressBER::IPAddressBER(IPAddress)'
  435 |     IPAddressBER(IPAddress value);
      |                  ~~~~~~~~~~^~~~~

Used library Version Path
Ethernet     2.0.0   /home/cydanil/.arduino15/packages/esp32/hardware/esp32/3.0.1/libraries/Ethernet
Networking   1.0.0   /home/cydanil/.arduino15/packages/esp32/hardware/esp32/3.0.1/libraries/Network
SPI          2.0.0   /home/cydanil/.arduino15/packages/esp32/hardware/esp32/3.0.1/libraries/SPI
WiFi         2.0.0   /home/cydanil/.arduino15/packages/esp32/hardware/esp32/3.0.1/libraries/WiFi
WebServer    2.0.0   /home/cydanil/.arduino15/packages/esp32/hardware/esp32/3.0.1/libraries/WebServer
FS           2.0.0   /home/cydanil/.arduino15/packages/esp32/hardware/esp32/3.0.1/libraries/FS
SNMP         1.4.0   /home/cydanil/Arduino/libraries/SNMP

Used platform Version Path
esp32:esp32   3.0.1   /home/cydanil/.arduino15/packages/esp32/hardware/esp32/3.0.1

An easy fix for me is to modify BER.cpp, line 422 with something like: ber = new IPAddressBER((uint32_t) 0);

I appreciate that this may not be the target platform, but I'm happy to contribute a solution if you give me some guidance :slightly_smiling_face:

Thanks,
Cyril

patricklaf commented 5 months ago

Hello Cyril, Thank you for reporting the issue. This problem appeared with ESP32 3.0.x cores, but this is non sense to pass a null pointer as an IPAddress object. Nevertheless, I prefer this solution:

ber = new IPAddressBER(IPAddress()); 

It compiles on all the platforms (AVR, ESP32 and STM32). Feel free to test it and if it works well for you, you can create a pull request if you want. Regards, Patrick

cydanil commented 5 months ago

I indeed noticed the issue following upgrades of the arduino-esp toolchain to 3+.

I tested that with both ESP32 and STM32 hardware on my test setup, and it works as expected.
I'll create an MR with the change.

Thanks for your time,
Cyril

patricklaf commented 5 months ago

PR #10 merged. I let you close this issue. Best regards, Patrick

cydanil commented 5 months ago

Sweet, thanks @patricklaf !