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

EasyConnect Seperation of Concerns Refactor #204

Merged
merged 5 commits into from
Mar 5, 2025

Conversation

bcarlson-dev
Copy link
Contributor

  • Separated Controller, Proxy Agent, and Enrollee into different classes for better code understanding
  • Introduced ec_manager

@bcarlson-dev bcarlson-dev requested a review from tuckerpo-cl March 5, 2025 19:09
@bcarlson-dev bcarlson-dev force-pushed the feature/ec-oop-refactor branch from 7450d20 to 5e57ad2 Compare March 5, 2025 19:37
Copy link
Contributor

@tuckerpo-cl tuckerpo-cl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stylistically I'd like to see override used in subclasses where applicable, just makes reasoning about the code easier for instance if you're reading a subclass header, immediately obvious that a given function extends some other base class. Also, regarding send_action_frame as a param for ec_manager, would that be bound to send_frame, or some new entry into em_agent which sends action frames via the bus to onewifi? Other than this comment and the other 2 left, LGTM.

Signed-off-by: Ben Carlson <[email protected]>
@bcarlson-dev
Copy link
Contributor Author

Stylistically I'd like to see override used in subclasses where applicable, just makes reasoning about the code easier for instance if you're reading a subclass header, immediately obvious that a given function extends some other base class. Also, regarding send_action_frame as a param for ec_manager, would that be bound to send_frame, or some new entry into em_agent which sends action frames via the bus to onewifi? Other than this comment and the other 2 left, LGTM.

I added the override and totally agree. I meant to do it but got mixed up in the fact that C++ doesn't require it.

Regarding the send_action_frame that'll need to be a new function definition in em_agent that utilizes the OneWifi "Device.WiFi.AccessPoint.{i}.RawFrame.Mgmt.Action.Tx" call.

@bcarlson-dev bcarlson-dev merged commit 4df23d1 into rdkcentral:main Mar 5, 2025
4 checks passed
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.

2 participants