-
Notifications
You must be signed in to change notification settings - Fork 182
fix TC_042_1_CS and TC_043_1_CS / local auth list not supported #425
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?
fix TC_042_1_CS and TC_043_1_CS / local auth list not supported #425
Conversation
…se it uses a deprecated version of `actions/cache: v2` Please update your workflow to use v3/v4 of actions/cache to avoid interruptions. Learn more: https://github.blog/changelog/2024-12-05-notice-of-upcoming-releases-and-breaking-changes-for-github-actions/#actions-cache-v1-v2-and-actions-toolkit-cache-package-closing-down
matth-x
left a comment
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.
@razvanphp Thanks again for the fixes (and for updating the CI)! Only one question, then this is good to go
|
|
||
| #if MO_ENABLE_LOCAL_AUTH | ||
| if (authorizationService) { | ||
| if (authorizationService && authorizationService->localAuthListEnabled()) { |
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.
This affects the SupportedFeatureProfiles Configuration key. In my understanding, that key tells the server what the charger is in theory capable of. When the the server disables unused profiles, then the charger would still be able to support them, in theory.
Did this change come out of an OCTT test case?
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.
Yes, when disabled, the OCTT expects it not to appear in the list anymore (as it is not supported). This avoids having another precompiled firmware just to pass those tests...
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.
Oh wow.
Then one tiny request: can you change LocalAuthListEnabled to reboot required?
BTW, the memory heap calculation misses some ENV variables I think, I can't fix that one. Do you still have access to those servers or should we look into another cloud-native solution? |
Thanks for looking into this too! The motivation to use a self-hosted CICD server was because the OCTT expected the SUT to expose endpoints on the internet. However they added a new connection method so that the SUT can just create another WebSocket connection to the server, alongside the OCPP connection. So the heap measurement pipeline can fully run on the GitHub Actions servers again and the self-hosted server can be removed. Already worked on something, just need to make it available on main |
Should I add unit tests for those? TBH, those are just for the OCTT test cases to pass... so probably not.
fixes #327