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

utcaApp: remove usage of sentinel parameters. #48

Merged
merged 1 commit into from
Nov 19, 2024
Merged

Conversation

ericonr
Copy link
Member

@ericonr ericonr commented Oct 28, 2024

This is a code cleanup, to avoid having to expose
UDriver::parameter_props to specific drivers.

The cleanup moves all handling of special cases (decoder_controller and read_only) into the ParamInit class, instead of requiring passing UDriver static inline members to it. This allowed us to add a ParamInit::set_dc() function, which is now used by the drivers which previously had to access parameter_props directly. This also simplifies the UDriver constructor, since it can now determine the need for p_tmp more easily, as well as the parameter properties.

It also allowed us to add more sanity checks, in this case to UDriver::writeGeneral(), to report writes into read-only parameters.


As suggested by Henrique in #45 (comment)

@gustavosr8

This is a code cleanup, to avoid having to expose
UDriver::parameter_props to specific drivers.

The cleanup moves all handling of special cases (decoder_controller and
read_only) into the ParamInit class, instead of requiring passing
UDriver static inline members to it. This allowed us to add a
ParamInit::set_dc() function, which is now used by the drivers which
previously had to access parameter_props directly. This also simplifies
the UDriver constructor, since it can now determine the need for p_tmp
more easily, as well as the parameter properties.

It also allowed us to add more sanity checks, in this case to
UDriver::writeGeneral(), to report writes into read-only parameters.
Copy link
Contributor

@henriquesimoes henriquesimoes left a comment

Choose a reason for hiding this comment

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

I didn't notice anything missing. Looks nice and clean.

Good to know we can even get further sanity checks with this design change. ;)

utcaApp/src/udriver.h Show resolved Hide resolved
@ericonr ericonr merged commit 3d12a00 into master Nov 19, 2024
2 checks passed
@ericonr ericonr deleted the props-remove-hack branch November 19, 2024 18:11
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