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

adrv9002 plugin fix for disabled channels #477

Merged
merged 6 commits into from
Feb 21, 2024
Merged

adrv9002 plugin fix for disabled channels #477

merged 6 commits into from
Feb 21, 2024

Conversation

andrei47w
Copy link
Contributor

The profile gen was failing to create a configuration since some attributes cannot be read if the channel is disabled

plugins/adrv9002.c Outdated Show resolved Hide resolved
plugins/adrv9002.c Show resolved Hide resolved
plugins/adrv9002.c Outdated Show resolved Hide resolved
plugins/adrv9002.c Outdated Show resolved Hide resolved
plugins/adrv9002.c Outdated Show resolved Hide resolved
plugins/adrv9002.c Outdated Show resolved Hide resolved
plugins/adrv9002.c Show resolved Hide resolved
plugins/adrv9002.c Show resolved Hide resolved
plugins/adrv9002.c Show resolved Hide resolved
plugins/adrv9002.c Show resolved Hide resolved
nunojsa
nunojsa previously approved these changes Feb 8, 2024
Copy link
Contributor

@nunojsa nunojsa left a comment

Choose a reason for hiding this comment

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

Some nits... I'm approving the PR but I do hope that at some point you get rid of CHANNEL_COUNT.

Moreover, the plugin source is getting way too complex (and big). It might make sense to detach the profile_gen from the main .c file.

plugins/adrv9002.c Outdated Show resolved Hide resolved
plugins/adrv9002.c Outdated Show resolved Hide resolved
nunojsa
nunojsa previously approved these changes Feb 15, 2024
plugins/adrv9002.c Outdated Show resolved Hide resolved
plugins/adrv9002.c Outdated Show resolved Hide resolved
@nunojsa nunojsa dismissed their stale review February 15, 2024 11:15

I mistakenly approved it... With the above changes, I'll approve it

…hannel status

- adrv9002_channel_is_enabled(struct adrv9002_common *chan) will return true or false

Signed-off-by: Andrei Popa <andrei.popa@analog.com>
- used adrv9002_channel_is_enabled() helper function
- some attributes cannot be read if channel is disabled, so they are set to 0
- improve warning messages

Signed-off-by: Andrei Popa <andrei.popa@analog.com>
- split RX and TX for loop

Signed-off-by: Andrei Popa <andrei.popa@analog.com>
… is not enabled

Signed-off-by: Andrei Popa <andrei.popa@analog.com>
Copy link
Contributor

@nunojsa nunojsa left a comment

Choose a reason for hiding this comment

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

Despite my comments (points 1 and 2), I'll still approve this. But if you still want to make those changes, that would be great

plugins/adrv9002.c Show resolved Hide resolved
… and removed for() loops

- removed the for() loop by implicit memcpy() in lte_lvs_3072_MHz_10() to match lte_defaults() definition style

Signed-off-by: Andrei Popa <andrei.popa@analog.com>
- replaced it with ADRV9002_NUM_CHANNELS which has the same value
- remove implied size for lists in profile_gen_update_channels()

Signed-off-by: Andrei Popa <andrei.popa@analog.com>
@andrei47w andrei47w merged commit 052621d into main Feb 21, 2024
10 checks passed
@andrei47w andrei47w deleted the adrv9002-fix branch February 21, 2024 13:38
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.

None yet

2 participants