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

Misc fixes #541

Merged
merged 11 commits into from
Apr 3, 2023
Merged

Misc fixes #541

merged 11 commits into from
Apr 3, 2023

Conversation

andlaus
Copy link
Member

@andlaus andlaus commented Mar 31, 2023

This PR contains a bunch of smaller fixes and improvements for stuff which I recently stumbled over and which are IMO not enough to open a pull request on their own. Probably the change with most potential for controversy is 298d2c8, which causes the list tool to treat signal.{initial,invalid,minimum,maximum} as physical values. (The ARXML loader has been doing this for quite some time by now, the other formats will hopefully be switched to using physical quantities for all user-facing values soon.)

Andreas Lauser <andreas.lauser@mercedes-benz.com>, on behalf of MBition GmbH.
Provider Information

this is for the benefit of people who use the VSCode IDE...

Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io>
Signed-off-by: Gerrit Ecke <gerrit.ecke@mbition.io>
This skips printing anything that relies on format-specific
information. The main purpose of this is to be able to check if two
files using different formats represent the same database.

Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io>
Signed-off-by: Gerrit Ecke <gerrit.ecke@mbition.io>
If applicable, we now print the unit for the pre-defined signal values
(because these are physical values) and we also alow to specify the
format specifier for physical signal values in the list parser. The
latter change allows to get around rounding errors when comparing
databases stemming from conversion of one file format to another.

Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io>
Signed-off-by: Gerrit Ecke <gerrit.ecke@mbition.io>
this is IMO less confusing as `signal.type` only specifies the type
which is used to internally represent the signal. In the physical
world, all signals are either floating point values or enumerations.

Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io>
Signed-off-by: Gerrit Ecke <gerrit.ecke@mbition.io>
the first signal is the current speed of the vehicle to be displayed
by the instrument cluster, the second is the target speed for the
cruise control ECU. So far, both were called "SpeedKm"...

Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io>
Signed-off-by: Gerrit Ecke <gerrit.ecke@mbition.io>
Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io>
Signed-off-by: Gerrit Ecke <gerrit.ecke@mbition.io>
The system signal is supposed to be transported by a given interaction
signal, i.e. to get a "global" state of the system, one must use the
system signals which can be transported by multiple interaction
signals of multiple PDUs...

Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io>
Signed-off-by: Gerrit Ecke <gerrit.ecke@mbition.io>
…time of multiplexed messages

cantools' data model currently does not support specifying cycle times
for a subset of a message signals different from the cycle time of the
message but ARXML does. (Personally, I much prefer the simplicity of
cantools, but we need to somehow handle this case...)

Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io>
Signed-off-by: Gerrit Ecke <gerrit.ecke@mbition.io>
such PDUs do not exist in AR4, but AR3 specifies non-signal bearing
PDUs this way...

Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io>
Signed-off-by: Gerrit Ecke <gerrit.ecke@mbition.io>
@coveralls
Copy link

coveralls commented Mar 31, 2023

Pull Request Test Coverage Report for Build 4595714280

  • 53 of 63 (84.13%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.02%) to 94.038%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cantools/database/can/formats/arxml/system_loader.py 9 13 69.23%
cantools/subparsers/list.py 44 50 88.0%
Totals Coverage Status
Change from base Build 4369265029: -0.02%
Covered Lines: 7051
Relevant Lines: 7498

💛 - Coveralls

@andlaus
Copy link
Member Author

andlaus commented Apr 3, 2023

@zariiii9003 or @juleq: can you have a look at this? For this PR I recommend looking at the patches individually as they are all pretty small and should be self-contained. (in principle, each of them could be done as a separate PR...)

@zariiii9003
Copy link
Collaborator

Yes, i wanted to get #536 merged first, i'll get that done today, i think.

thanks goes to [at]zariiii9003

Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io>
Signed-off-by: Gerrit Ecke <gerrit.ecke@mbition.io>
turns out that f-strings are sufficient after all. many thanks to
[at]zariiii9003 for the hint!

Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io>
Signed-off-by: Gerrit Ecke <gerrit.ecke@mbition.io>
@andlaus andlaus merged commit 48cf15c into cantools:master Apr 3, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants