Cannot call `setUrl` more than once. Original data always returned

spouliot commented 3 years ago

Basic Infos

[x] I have tested that the issue is present in current master branch (aka latest git).
[x] I have searched the issue tracker for a similar issue.
[x] If there is a stack dump, I have decoded it.
[x] I have filled out all fields below.

Platform

Hardware: ESP32
Core Version: not sure
Development Env: PlatformIO
Operating System: macOS

Settings in IDE

Module: Espressif 32 (2.1.0) > Espressif ESP32 Dev Module
Flash Mode: ?
Flash Size: 4MB
lwip Variant: ?
Reset Method: RTS
Flash Frequency: ?
CPU Frequency: 240MHz
Upload Using: SERIAL
Upload Speed: 115200

Problem Description

Since the constructor does not accept any argument it seems that the parser should be re-usable. IOW I expected to be able to call setUrl several times and get the correct data out of it.

However it seems that the code always return the first URL's data. If that's the expected behaviour then I do not think setUrl should return true.

Of course I'd prefer it to be reusable :)

MCVE Sketch


#include <Arduino.h>

void setup() {

}

void loop() {
  LCBUrl parser;
  if (parser.setUrl ("http://192.168.2.1/")) {
    Serial.printf ("Host 1: %s\n", parser.getHost().c_str());
  }
  if (parser.setUrl ("http://192.168.2.2/")) {
    Serial.printf ("Host 2: %s\n", parser.getHost().c_str());
  }
  if (parser.setUrl ("")) {
    // that returns true too!
    Serial.printf ("Host 3: %s\n", parser.getHost().c_str());
  }
  delay (5000);
}

Debug Messages

Host 1: 192.168.2.1
Host 2: 192.168.2.1
Host 3: 192.168.2.1
...

lbussy commented 3 years ago

Hm - I had not considered using the same instance to change the URL. This is how I intended that it be used:

  LCBUrl parser1;
  LCBUrl parser2;
  if (parser1.setUrl ("http://192.168.2.1/")) {
    Serial.printf ("Host 1: %s\n", parser.getHost().c_str());
  }
  if (parser2.setUrl ("http://192.168.2.2/")) {
    Serial.printf ("Host 2: %s\n", parser.getHost().c_str());
  }

I can look to see if I can quickly make it re-assignable. In the meantime you could probably do something like:

  LCBUrl parser;
  if (parser.setUrl ("http://192.168.2.1/")) {
    Serial.printf ("Host 1: %s\n", parser.getHost().c_str());
  }
  delete parser;
  LCBUrl parser;
  if (parser.setUrl ("http://192.168.2.2/")) {
    Serial.printf ("Host 2: %s\n", parser.getHost().c_str());
  }
  delete parser;
  LCBUrl parser;
  if (parser.setUrl ("")) {
    // that returns true too!
    Serial.printf ("Host 3: %s\n", parser.getHost().c_str());
  }

Not optimal, but unless you are working on an Uno, space is likely not a concern?

I will have a look though.

spouliot commented 3 years ago

It's not a big deal, now that I know it, and I can create a new instance each time.

But it's not what I had expected either. It was surprising to see everything calling the same endpoint. So I decided to file an issue :)

If not reusable then I would have expected something like:

LCBUrl parser("http://192.168.2.1/");
Serial.printf("Host: %s\n", parser.getHost().c_str());

or setUrl to return false on additional calls.

BTW huge thanks for sharing this library and the quick replies!

lbussy commented 3 years ago

I needed it, figured other people would too. Makes sense as a companion to the microcontroller-based projects we do.

I appreciate the very thorough issue report!

lbussy commented 3 years ago

Okay this is now in devel. If you get a chance to test this and 13 let me know?

lbussy / LCBUrl