-
-
Notifications
You must be signed in to change notification settings - Fork 555
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
eerimoq
merged 2 commits into
cantools:master
from
mercedes-benz:arxml_consider_all_packages
Feb 1, 2021
Merged
ARXML: Recursively check all AR-PACKAGES for CAN clusters #263
eerimoq
merged 2 commits into
cantools:master
from
mercedes-benz:arxml_consider_all_packages
Feb 1, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will probably break at some point as there is no test. Should I merge anyway? |
I guess it's a bit unlikely because this code path is excercised even for flat files?
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
force-pushed
the
arxml_consider_all_packages
branch
from
February 1, 2021 11:56
7a67c96
to
540e078
Compare
ok. should be good to go now... |
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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