-
Notifications
You must be signed in to change notification settings - Fork 189
TC_068_CS / Stop transaction logic simplication #433
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
base: main
Are you sure you want to change the base?
Conversation
85e1d93 to
22f9dd7
Compare
|
I feel like we should add unit test for those cases, especially if OCTT has them. |
|
|
||
| //check for active transactions | ||
| for (unsigned int cId = 1; cId < context->getModel().getNumConnectors(); cId++) { | ||
| if (isTransactionActive(cId)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to be honest, this is not really needed, as it's checked again in endTransaction()...
| if (isTransactionActive(cId)) { | ||
| //this will check both parentIdTag and idTag | ||
| if (endTransaction(idTag, "Local", cId)) { | ||
| MO_DBG_INFO("ended active transaction on connector %d for idTag '%s'", cId, idTag); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and this log is also duplicated in endTransaction(), so I could slim this down a lot. What do you think?
|
@razvanphp In the ESP-IDF example, I'm missing a bit of context here: when is Generally I like the idea of having a function which handles NFC card swipes and you don't need to implement that begin-or-endTransaction logic yourself anymore. That should probably be a new function like |
Dear matth: I have meet similar problem with nfc card.For example, your library implements reservation processing, but the example will contradict the library's implementation. For instance, the example does not support the parentID part. if you could give us a new example that is very impressive |
@matth-x what do you think about this? We found this removed a lot of lines of code from our main loop.
If you like the idea, I can add a test-case for it in unit tests.
Still, this test case fails for some reason I can't understand:
so the charger sends the
StatusNotification, but afterStartTransactionRequest, isn't this expected?