[DP] Refactor away from diagnosticProtocolList #281
Conversation
GwnDaan
temporarily deployed
to
Integrate Pull Request
GitHub Actions
Inactive
I'll do what I can, I have some travel coming up soon-ish but I might be able to make headway before then. I have a reliable way to test this interface. I think generally it's probably a good idea to make this more like our other interfaces. It needed some updates to be sure, especially to support #161. |
I see what you mean here, and I agree, but as far as I can see it never has been not optional? As one still needed to use the |
Totally agree, that was a shortcoming in the old implementation, so I had made it a protocol to get automatic updates from the stack but failed to make it truly automatic... I am not sure if we can make the network manager really handle it, unless it automatically attaches an instance of this class to every ICF, which is still not super intuitive then that users need to configure it... plus the manager is already so big... I guess for the moment I'm happy enough with making it like our other, nicer interfaces if you are. Side note, I am working unit tests for this in my evening free time and found some other smaller issues that I'll open as other PRs soon... |
Fixed an issue where we'd incorrectly allow multiple BAM Tx sessions at one time for a given ICF, which is not allowed by the standard. The issue here was that when we were checking for extant sessions, we were calling only the destination specific session getter.
Fixed order of operations when nulling out an evicted CF.
- Stop using CANLibProtocol to be more inline with newly added interfaces (Guidance/speed/maintain power) - Add dedicated NetworkType configuration parameter - use in-class initializers where possible - Fixed several typos - Moved `parse_j1939_network_states` function from public to private - Introduced `initialize` / `terminate` as replacements for `assign_...` / `deassign_....`
- Remove redundant case ' clauses' - Remove redundant parenthesis These changes were suggested by SonarCloud
Added numerous DP unit tests. Fixed DP example not saving the periodic event dispatcher. Added example use of ECUID Type. Added a way to get the broadcast state and init state of the diagnostic protocol. Added processing of the suspend type for DM13 to allow for custom broadcast suspension times. Added support for PGN requests for the DM1 PGN. Fixed issues with ECU ID sending too many fields when operating in J1939 mode. Enhanced message queuing for requests for ECU and software ID. Fixed busload unit test which was affected by adding DP tests. Slightly increased the timeout on language command timestamp unit test. Fixed an intermittent issue with TC unit tests that came from changing to shared ptr for control functions due to the tests claiming the same address with no network manager updates between destroying a partner and claiming a new one.
ad3154
force-pushed
the
improve-diagnostics-protocol
branch
from
b51350d to
0c269c9
Compare
Added additional diagnostic protocol tests. Fixed issues with DTC getters returning false all the time.
ad3154
temporarily deployed
to
Integrate Pull Request
GitHub Actions
Inactive
Added some additional unit tests to diagnostic protocol to catch a few more cases with DM2, 22, and 13. Added a missing doxygen comment in ControlFunction's constructor.
ad3154
temporarily deployed
to
Integrate Pull Request
GitHub Actions
Inactive
|
SonarCloud Quality Gate failed.
|
|
Yeah LGTM, can be merged :) |
Changes:
Major:
initialize/terminateas replacements forassign_.../deassign_....Minor:
parse_j1939_network_statesfunction from public to private@ad3154, note that I don't have any way of testing if this actually works. I just refactored up to a point where I think I can better fix #161. Also we need to expand on the unit test cases for this diagnostic protocol, but I'm not really familiar with the documentation. Any chance you have time to take this over?