ebroecker / canmatrix

Converting Can (Controller Area Network) Database Formats .arxml .dbc .dbf .kcd ...
BSD 2-Clause "Simplified" License
908 stars 401 forks source link

Loading ARXML: bool' object (speed) has no attribute 'text' #448

Closed ForestRupicolous closed 3 years ago

ForestRupicolous commented 4 years ago

When I'm loading a ARXML with canmatrix.formats.loadp_flat(sys.argv[1]) I get an AttributeError: 'bool' object has no attribute 'text' for the logger output in line 1704 of arxml.py.

In line 1690 speed is read in as None. In line 1694 it is set to false (I don't understand the goal behind this line). In line 1704 the speed.text is read but not available.

                logger.debug(" Speed: " + speed.text)
ebroecker commented 4 years ago

Hi @ForestRupicolous , thanks for you report.

there are two possible elements which could tell us the bus speed:

<SPEED>

and

<BAUDRATE>

If one of them exists - use it as speed.

Line the logger.debug(" Speed... should be moved under the line in the if speed is not None:

Things more clear now?

ForestRupicolous commented 4 years ago

Wow, you are really fast in responding. Thank you! Things are more clear. I still think the code has an issue here. I now moved the logger line like you suggested and still get the error message as SPEED and BAUDRATE are None and therefore speed is set to False with

            speed = baudrate_elem is speed is None

as if speed is not None: is True (False != None)

My next step is to figure out why I get no Baudrate as it is defined in my .Arxml.

When I comment the log message out I'm able to generate a db.

EDIT: BAUDRATE is not none but _<Element {http://autosar.org/schema/r4.0}BAUDRATE at 0x(LONG HEXVALUE)> Still the value of speed is False

ForestRupicolous commented 4 years ago

I fixed it for my case by commenting out speed = baudrate_elem is speed is None and adding

            elif baudrate_elem is not None:
                db.baudrate = baudrate_elem.text
            logger.debug("Baudrate: "+ db.baudrate)

For db.baudrate, should it be the XML-Element or the Baudrate value?

            if speed is not None:
                db.baudrate = speed  # here the xml element is used       

            if fd_baudrate_elem is not None:
                db.fd_baudrate = fd_baudrate_elem.text # here the text is used
ebroecker commented 4 years ago

You are probably right that there's an issue in the code. I'll have to look a bit deeper

ForestRupicolous commented 4 years ago

If created a branch with the fix that worked for me: https://github.com/ebroecker/canmatrix/compare/development...ForestRupicolous:arxml-read-baudrate Unfortunately it looks like the baudrate definition is completely different in my .ARXML (I can't share) and in your test.arxml. So my branch doesn't pass your tests. Additionally I had to make 2 small fixes to read in hex values defined with 0x prefix which is not really to focus of the issue here.

As example I can share the part where the baudrate is defined:

<AR-PACKAGE UUID="XXXX">
      <SHORT-NAME>Topology</SHORT-NAME>
      <AR-PACKAGES>
        <AR-PACKAGE UUID="XXXX">
          <SHORT-NAME>Clusters</SHORT-NAME>
          <ELEMENTS>
            <CAN-CLUSTER UUID="XXXX">
              <SHORT-NAME>Diagnostics</SHORT-NAME>
              <CAN-CLUSTER-VARIANTS>
                <CAN-CLUSTER-CONDITIONAL>
                  <BAUDRATE>500000</BAUDRATE>
                  <PHYSICAL-CHANNELS>
                    <CAN-PHYSICAL-CHANNEL UUID="XXXX">
                      <SHORT-NAME>Device_CAN</SHORT-NAME>
                    </CAN-PHYSICAL-CHANNEL>
                  </PHYSICAL-CHANNELS>
                  <CAN-FD-BAUDRATE>0</CAN-FD-BAUDRATE>
                </CAN-CLUSTER-CONDITIONAL>
              </CAN-CLUSTER-VARIANTS>
            </CAN-CLUSTER>
....
ebroecker commented 4 years ago

Thanks for this snippet and for your branch.

give me some time to understand the differences. Maybe my ARXML is wrong and also my test. I have to investigate...

ebroecker commented 4 years ago

https://github.com/ebroecker/canmatrix/blob/60c61f4f395222ccad98f45c3b73b773cc6db8a9/src/canmatrix/tests/ARXML_min_max.arxml#L58

doesn't look to different does it?

ForestRupicolous commented 4 years ago

You are right. I think I justed used a complete wrong file to test against in this case. Have to check again tomorrow with the correct one.

ForestRupicolous commented 4 years ago

OK, I now found the time to check this: When I'm trying to run canmatrix\test\test.py my code fails as it has some issues parsing canmatrix\test\test.arxml. With _canmatrix\src\canmatrix\tests\ARXML_minmax.arxml everything looks fine. Is the canmatrix\test\test.arxml still vaild?

ebroecker commented 4 years ago

test.arxml is maybe outdated , but usually it should work.

I had a look into your changes. Therfor I discovered that CanMatrix.baudrate should be int.

so with following changes test.arxml works again:

            if speed is not None:
                db.baudrate = int(speed.text)
            elif baudrate_elem is not None:
                db.baudrate = int(baudrate_elem.text)

Why did you change this line: https://github.com/ForestRupicolous/canmatrix/blob/arxml-read-baudrate/src/canmatrix/formats/arxml.py#L922

Did you have an issue with it? Could you create a pull-request ?

Thanks

ForestRupicolous commented 4 years ago

I changed the line (and the one in canmatrix.py, see https://github.com/ForestRupicolous/canmatrix/commit/373062c7681add8230ed87ccbc8f48854a693ff9) to be able to read in hex values defined with the prefix 0x. I'm aware that there might be a cleaner solution using a different float_factory. I'm just not sure how or which.

I will adapt your changes and create a pull-request when I find time next week.

ebroecker commented 4 years ago

perfect