[go: nahoru, domu]

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

ARXML: Recursively check all AR-PACKAGES for CAN clusters #263

Merged
merged 2 commits into from
Feb 1, 2021

Conversation

andlaus
Copy link
Member
@andlaus andlaus commented Feb 1, 2021

Currently just the top level packages are checked. I recently stumbled over a file in the wild which needs this atrocity.

This PR is a replacement for #174 and it hopefully completes the refactoring of the ARXML code.

Andreas Lauser andreas.lauser@mbition.io, Mercedes-Benz AG on behalf of MBition GmbH.

Imprint

@coveralls
Copy link
coveralls commented Feb 1, 2021

Coverage Status

Coverage increased (+0.007%) to 97.051% when pulling 540e078 on Daimler:arxml_consider_all_packages into 03b1a3c on eerimoq:master.

@eerimoq
Copy link
Collaborator
eerimoq commented Feb 1, 2021

This will probably break at some point as there is no test. Should I merge anyway?

@andlaus
Copy link
Member Author
andlaus commented Feb 1, 2021

This will probably break at some point as there is no test.

I guess it's a bit unlikely because this code path is excercised even for flat files?

Should I merge anyway?

Wait for now. I'll modify the ARXML3 test file.

instead of just the top level ones. I recently stumbled over a file in
the wild which needs this.
@andlaus
Copy link
Member Author
andlaus commented Feb 1, 2021

ok. should be good to go now...

@eerimoq eerimoq merged commit 52b1421 into cantools:master Feb 1, 2021
@andlaus andlaus deleted the arxml_consider_all_packages branch December 11, 2021 08:09
VonSquiggles pushed a commit to VonSquiggles/cantools that referenced this pull request May 13, 2022
…#263)

* ds402: Increase delay to check status on homing start.

The Statusword is examined immediately after setting the Controlword
command to start homing.  That is very likely to fail because of the
round-trip time until the Statusword is actually updated from a TPDO.
To work around that, the delay between each check of the Statusword
should be moved before the actual comparison, and its default value
increased.

Introduce a new constant TIMEOUT_CHECK_HOMING to configure that with a
default value of 100 ms.  This replaces the previously used
INTERVAL_CHECK_STATE which is only 10 ms by default.

An even better solution would be to wait for the Statusword to be
updated by a received PDO, but that would be much more complex.

* Apply interval to is_homed() method as well.

Same problem as in the homing() method, PDO updates of the Statusword
need at least one SYNC / PDO cycle duration.

* Factor out common _homing_status() method.

Move the common code from is_homed() and homing() to a method for
internal use.  Add a comment why the delay is necessary and how it
should possibly be replaced by an RPDO reception check.  Should the
latter be implemented, there will be only one place to change it.

* p402: Add blocking method check_statusword().

In contrast to the property getter, this method will not simply return
the cached last value, but actually wait for an updated TPDO
reception.  If no TPDO is configured, or it is not expected
periodically, the usual statusword getter takes over and returns
either the last received value or queries via SDO as a fallback.

The timeout for receiving a TPDO can be configured individually per
call, defaulting to 0.2 seconds (TIMEOUT_CHECK_TPDO).

* p402: Wait for TPDO instead of timed statusword checking.

Several methods loop around waiting for some expected statusword
value, usually calling time.sleep() with the INTERVAL_CHECK_STATE or
INTERVAL_CHECK_HOMING constants.  Replace that with a call to
check_statusword(), which will only block until the TPDO is received.

This allows for possibly shorter delays, because the delay can be cut
short when the TPDO arrives in between.  But rate limiting the
checking loops is still achieved through the blocking behavior of
check_statusword().

If no TPDO is configured, the statusword will fall back to checking
via SDO, which will also result in periodic checks, but only rate
limited by the SDO round-trip time.  Anyway this is not a recommended
mode of operation, as the warning in _check_statusword_configured()
points out.

* p402: Fix wrong reports of INTERRUPTED homing procedure.

While the statusword checks work properly now, there is no delay
anymore to make sure the controlword RPDO which starts the homing
procedure is already received before we start checking the homing
status.  So the first reading might easily return INTERRUPTED, meaning
that the controlword change has just not been applied yet.

Add one extra call for check_statusword() to wait for one extra cycle
in case of periodic transmissions.  In effect, the success checking
logic starts looking at the second received statusword after setting
the controlword.

Co-authored-by: André Filipe Silva <af-silva@users.noreply.github.com>
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