JoelBender / BACpypes3

BACnet communications library
33 stars 7 forks source link

read_property returns None if error #15

Open ChristianTremblay opened 8 months ago

ChristianTremblay commented 8 months ago

This is not necessarely a bug but it prevents BAC0 from knowing "why" as the exception is dealt internally in bacpypes3. All we get is None.

In BAC0 I used that to fill out a default value for description, inactiveText and activeText that are not always implemented in old devices.

JoelBender commented 8 months ago

There shouldn't be any completely silent exceptions, the method should return an error/reject/abort instance or raise the exception. Please describe the condition that should be returning/raising the problem.

ChristianTremblay commented 8 months ago

image

See here, I get the variable description being set to None... I see that an error occurred, but only through a print statement.

JoelBender commented 8 months ago

If the description property is appropriate for the object but the property isn't assigned a value, it will be None in the object itself, which will be returned from the bacpypes2.object.Object.read_property() method. However, if it's not an attribute of the object, that method will raise an AttributeError.

On the server side, there was a bug in this code that did not capture the attribute error so an "operational problem" was being returned to the client, which I just fixed in 0.0.82. I also updated the do_read() method to allow for integer property identifiers.

I think the "property: unknown-property" that you are seeing in your shell (the line after the [4]) is because the exception is being caught and printed, so the value being returned is really undefined. Try with a --debug bacpypes3.service.object.ReadWritePropertyServices and see what's going on in the read_property() method.

ChristianTremblay commented 8 months ago

In bp2, I was getting a iocb.ioError. In which I was able to find the "reason". In that case, I was explicitly dealing with properties I needed to have a value in.

# Extract from BAC0 using bp2
if iocb.ioError:  # unsuccessful: error/reject/abort
            apdu = iocb.ioError
            reason = find_reason(apdu)
            if reason == "segmentationNotSupported":
                value = self._split_the_read_request(args, arr_index)
                return value
            else:
                if reason == "unknownProperty":
                    if "description" in args:
                        self._log.warning(
                            "The description property is not implemented in the device. "
                            "Using a default value for internal needs."
                        )
                        return "Property Not Implemented"
                    elif "inactiveText" in args:
                        self._log.warning(
                            "The inactiveText property is not implemented in the device. "
                            "Using a default value of Off for internal needs."
                        )
                        return "Off"
                    elif "activeText" in args:
                        self._log.warning(
                            "The activeText property is not implemented in the device. "
                            "Using a default value of On for internal needs."
                        )
                        return "On"
                    else:
                        raise UnknownPropertyError("Unknown property {}".format(args))
                elif reason == "unknownObject":
                    self._log.warning("Unknown object {}".format(args))
                    raise UnknownObjectError("Unknown object {}".format(args))
                elif reason == "bufferOverflow":
                    self._log.warning(
                        "Buffer capacity exceeded in device {}".format(args)
                    )
                    return self._split_the_read_request(args, arr_index)
                else:
                    # Other error... consider NoResponseFromController (65)
                    # even if the real reason is another one
                    raise NoResponseFromController(
                        "APDU Abort Reason : {}".format(reason)
                    )

Now bp3 returns None. Not an exception. The error is not sent out of the boundary of bp3