OpenZWave / open-zwave

a C++ library to control Z-Wave Networks via a USB Z-Wave Controller.
http://www.openzwave.net/
GNU Lesser General Public License v3.0
1.05k stars 911 forks source link

Crash in meter, (possibly )caused by PAN08-1 In Wall Roller Shutter Controller #1842

Closed petergebruers closed 5 years ago

petergebruers commented 5 years ago

Reported on Domoticz forum:

https://github.com/domoticz/domoticz/issues/3326

Logs in that issue. The gist:

2019-06-16 16:57:05.828  Error: #5  0x76cdb730 in _IO_vfprintf_internal (s=s@entry=0x6e4fdd20, format=format@entry=0x9a29e8 "Can't Find a ValueID Index for %s with Unit %s (%d) - Index %d", ap=..., ap@entry=...) at vfprintf.c:1642
2019-06-16 16:57:05.828  Error: #6  0x76d0175c in _IO_vsnprintf (string=0x6e4fde98 "Can't Find a ValueID Index for Electric - W with Unit ", maxlen=<optimized out>, format=0x9a29e8 "Can't Find a ValueID Index for %s with Unit %s (%d) - Index %d", args=...) at vsnprintf.c:119
2019-06-16 16:57:05.828  Error: #7  0x006d92cc in OpenZWave::Internal::Platform::LogImpl::Write(OpenZWave::LogLevel, unsigned char, char const*, std::__va_list) ()
2019-06-16 16:57:05.828  Error: #8  0x006d5e6c in OpenZWave::Log::Write(OpenZWave::LogLevel, unsigned char, char const*, ...) ()
2019-06-16 16:57:05.829  Error: #9  0x00722f04 in OpenZWave::Internal::CC::Meter::HandleReport(unsigned char const*, unsigned int, unsigned int) ()

So it crashed while printing a log message "Can't Find a ValueID Index for Electric - W with Unit" and then something probably invalid.

This happened after receiving data from Node 5:

Node005, Loading Cache for node 5: Manufacturer=Philio Technology Corp, Product=PAN08-1 In Wall Roller Shutter Controller

(...)

Node005, Received Meter Report for Electric - W (1) with Units W (2) on Index 2: 0.5

I am not sure why the poster's log claims 1.6-166-gc4f72caa, it has got to be a more recent version because iirc that version did not have namespaces like OpenZWave::Internal::CC::Meter::HandleReport as visible in the stack trace - I am very confused by this contradiction.

Fishwaldo commented 5 years ago

Simple Fix. We didn't pass the correct number of Params to our Log::Write call.

Note - This happens when we are still interviewing the Device and have not yet created the ValueID's. In which case - we will bail out... (Only Version 1 of the CC will create a ValueID on receipt of a report - As Version 1 didn't have things like GetSupported and GetScaleSupported etc etc etc).

petergebruers commented 5 years ago

Thanks! I'll provide feedback on the Domoticz forum.

gvdhoven commented 5 years ago

@Fishwaldo could this also have happened in the SwitchMultiLevel class? In my case i also am using Domoticz, and while Domoticz is still querying the nodes, i send a swichmultilevel command. I just pulled in all changes and rebuilding domoticz; but here is the (SegFault) issue which occurs now, pointing to OZW: https://github.com/domoticz/domoticz/issues/3350

gvdhoven commented 5 years ago

Simple Fix. We didn't pass the correct number of Params to our Log::Write call.

Note - This happens when we are still interviewing the Device and have not yet created the ValueID's. In which case - we will bail out... (Only Version 1 of the CC will create a ValueID on receipt of a report - As Version 1 didn't have things like GetSupported and GetScaleSupported etc etc etc).

@Fishwaldo could this also have happened in the SwitchMultiLevel class? In my case i also am using Domoticz, and while Domoticz is still querying the nodes, i send a swichmultilevel command. Here is the (SegFault) issue which occurs now, pointing to OZW: https://github.com/domoticz/domoticz/issues/3350

b.t.w. I just pulled in all changes and rebuilding domoticz, will update you once i have more info

petergebruers commented 5 years ago

@Webunity this issue is about a bug in the Meter class, not MultiLevel. It has been fixed and the issue is closed. Different classes have different implementations. The symptoms may seem similar though.

I work with @Fishwaldo, I work on OpenZWave issues and I will handle your case and if I get your log file I'll make an issue on the OZW tracker for you, don't worry about that.

Fishwaldo commented 5 years ago

No. You need the dev branch to fix the multilevel crash.

gvdhoven commented 5 years ago

@petergebruers thanks and @Fishwaldo i have just pulled the dev branch; will update the original issue and not add 'waste' to this one. Sorry.

petergebruers commented 5 years ago

@Fishwaldo I haven't seen a detailed OZWLog but I understand what you are saying... It is about the reordering of the interview. Because @Webunity is already building (and Rpi Domoticz + OZW is slow to build) I'd like to get a log from him, then see if Dev branch fixes it.

@Webunity the Dev branch has not received much testing...

petergebruers commented 5 years ago

will update the original issue and not add 'waste' to this one. Sorry.

No need to apologize. I know you're stuck with a non-functional system right now.

Fishwaldo commented 5 years ago

The backtrack is the same as the interview issue we fixed in Dev Branch. So I’m willing to bet the same thing.

I’ve been hammering on testing the dev branch today. Seems ok. I will be doing some more testing soon on it make sure the security works well and then merge with master.

petergebruers commented 5 years ago

That would be awesome, if you could release it. I've been running Dev too and saw no issues, but then... I did not have devices that were affected.

Fishwaldo commented 5 years ago

Yeah. Just nervous. At least it passed a bunch of Qubino devices today that tend to do things slight differently

zzattack commented 5 years ago

My affected fibaro roller shutters regained functionality after recompiling from the dev branch. My input is just anecdotal for now. I can add before/after logfiles if that helps in getting this merged.

petergebruers commented 5 years ago

@zzattack thank you for testing the Dev branch. This is good news. Unless @Fishwaldo wants to check something specific, there is no need to send the log files.

Fishwaldo commented 5 years ago

I just got back from some business travels. I’ll try to merge this next week.