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

Ultrasonic Plugin _P013_HCSR04.ino broken with last commit of plugin #1518

Closed
lhurt opened this issue Jun 19, 2018 · 10 comments
Closed

Ultrasonic Plugin _P013_HCSR04.ino broken with last commit of plugin #1518

lhurt opened this issue Jun 19, 2018 · 10 comments
Labels
Category: Plugin Related to supported sensors Status: Fixed Commit has been made, ready for testing Type: Bug Considered a bug

Comments

@lhurt
Copy link

lhurt commented Jun 19, 2018

I use a self compiled version.

Commit dcf8e86 (Jun. 9th) broke plugin _P013_HCSR04.ino

Build with tag mega-20180606

_ works and shows valid values like "ULTRASONIC : Distance: 167.00".
Build with tag mega-20180611 doesn't work and shows "ULTRASONIC : TaskNr: 2 Distance: No reading!".

Expected behavior

The values should be retrieved again.

Actual behavior

No values are retrieved making the sensor useless.

Steps to reproduce

  1. Build firmware with tag mega-20180611 or later
  2. See log output

System configuration

Hardware: H801 (ESP 8266)

Log of working version (mega-20180606)
mega-20180606

Log of defective version (mega-20180611)
mega-20180611

@lhurt lhurt changed the title Ultrasonic Plugin _P013_HCSR04.ino broken with last commit Ultrasonic Plugin _P013_HCSR04.ino broken with last commit of plugin Jun 19, 2018
@TD-er TD-er added Type: Bug Considered a bug Category: Plugin Related to supported sensors labels Jun 19, 2018
@DittelHome
Copy link

Same here ....

@lhurt
Copy link
Author

lhurt commented Sep 13, 2018

Is it so hard to fix this? I use this pretty heavily and I am stuck with an old version.

@TD-er
Copy link
Member

TD-er commented Sep 13, 2018

I will revert it to an older version and do fix it later.
I wanted to test it myself, with a test setup, but I have been busy lately with other issues and the last 2 days trying to figure out why MQTTimporter crashes (hard).

@lhurt
Copy link
Author

lhurt commented Sep 17, 2018

I will revert it to an older version and do fix it later.
I wanted to test it myself, with a test setup, but I have been busy lately with other issues and the last 2 days trying to figure out why MQTTimporter crashes (hard).

Thanks for the quick reply.

@JojoS62
Copy link
Contributor

JojoS62 commented Sep 27, 2018

its also the same issue: #1575
I've tried to fix it, please check my comments in that issue.

@JojoS62
Copy link
Contributor

JojoS62 commented Sep 27, 2018

ok, I think I found the reason: the module is killing itself. There is a temporary oject created and inserted into a std::map. But the object is deleted on leaving the PLUGIN_INIT:

INIT : Booting version:  (ESP82xx Core 2_4_2, NONOS SDK 2.2.1(cfd48f3), LWIP: 2.0.3)
100 : INIT : Cold Boot - Restart Reason: External System
102 : FS   : Mounting...
126 : FS   : Mount successful, used 76304 bytes of 957314
151 : CRC  : No program memory checksum found. Check output of crc2.py
179 : CRC  : SecuritySettings CRC   ...OK 
272 : INIT : Free RAM:27376
273 : INIT : I2C
273 : INIT : SPI not enabled
277 : constructor P_013_sensordef
4278 : delete P_013_sensordef
4279 : ULTRASONIC : TaskNr: 1 TrigPin: 12 IRQ_Pin: 15 max dist cm: 150 max echo: 8607 nr_tasks: 1

so the constructor is called and after that the destructor. A copy of the struct is still in the map but pointing to an deleted NewPingESP8266 object. That caused also firmware reboots.

@TD-er
Copy link
Member

TD-er commented Sep 27, 2018

That sounds plausible.
I will have to look at the code to see why.

@JojoS62
Copy link
Contributor

JojoS62 commented Sep 27, 2018

here is the construction of a temporay object:

P_013_sensordefs[event->TaskIndex] =

I have uncommented the delete call in
delete sonar;
, then it works.
Maybe it is better to call delete in the PLUGIN_EXIT before the erase. Or not using a temporary object for inserting in the map.

@JojoS62
Copy link
Contributor

JojoS62 commented Sep 27, 2018

this will fix it in PLUGIN_INIT:

        P_013_sensordef *newSensordef = new P_013_sensordef(Plugin_013_TRIG_Pin, Plugin_013_IRQ_Pin, max_cm_distance);
        P_013_sensordefs[event->TaskIndex] = *newSensordef;

in PLUGIN_EXIT, the erase is called and this will call the destructor, so memory will be cleaned correctly.

I've created a PR for this fix.

@TD-er
Copy link
Member

TD-er commented Sep 29, 2018

Fixed by #1809

@TD-er TD-er closed this as completed Sep 29, 2018
@TD-er TD-er added the Status: Fixed Commit has been made, ready for testing label Sep 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Plugin Related to supported sensors Status: Fixed Commit has been made, ready for testing Type: Bug Considered a bug
Projects
None yet
Development

No branches or pull requests

4 participants