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

Enable Osc Plot #513

Open
wants to merge 8 commits into
base: libiio-v1-support
Choose a base branch
from
Open

Enable Osc Plot #513

wants to merge 8 commits into from

Conversation

cristina-suteu
Copy link
Contributor

@cristina-suteu cristina-suteu commented Jan 9, 2025

PR Description

Osc plot now uses libiio v1 API and data is captured with the iio_stream object.

PR Type

  • Bug fix (a change that fixes an issue)
  • New feature (a change that adds new functionality)
  • Breaking change (a change that affects other repos or cause CIs to fail)

PR Checklist

  • I have followed the coding standards and guidelines
  • I have conducted a self-review of my own code changes
  • I have commented new code, particulary complex or unclear areas
  • I have checked in CI output that no new warnings/errors got introduced
  • I have updated documentation accordingly (GitHub Pages, READMEs, etc)

osc.c Outdated
@@ -167,7 +167,7 @@ unsigned global_enabled_channels_mask(struct iio_device *dev, struct iio_channel
unsigned mask = 0;
int scan_i = 0;
unsigned int i = 0;
//struct iio_channels_mask *ch_mask = iio_get_channels_mask(iio_device_get_channels_count(dev));
//struct iio_channels_mask *ch_mask = iio_create_channels_mask(iio_device_get_channels_count(dev));
Copy link
Contributor

Choose a reason for hiding this comment

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

You forgot to uncomment the variable that stores the channels mask.

Copy link
Contributor

Choose a reason for hiding this comment

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

Still not addressed. Was it on purpose?

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.

@dNechita, @cristina-suteu, Can we move this forward? I think it's time for us (ADI) to really start using and pushing libiio v1. Having osc core done would be a very good start so that we could start updating individual plugins.

* that's because someone else broke the lock
*/
if (markers_copy) {
osc_plot_set_markers_copy(plot, NULL);
Copy link
Contributor

Choose a reason for hiding this comment

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

odd indentation...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'll fix the indentation in a final commit , after changes have been reviewed

@cristina-suteu
Copy link
Contributor Author

@nunojsa , @dNechita already ported most of the plugins, i need to to finish this up and then we can merge it as well. i've been caught with another project , but i'll take care of this by the end of the week

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.

As a general recommendation, no need to add the file suffix in the commit subject. Not sure if the final patch is going to be squashed but if not please give a better subject other than "final fixes"

osc.c Outdated
@@ -167,7 +167,7 @@ unsigned global_enabled_channels_mask(struct iio_device *dev, struct iio_channel
unsigned mask = 0;
int scan_i = 0;
unsigned int i = 0;
//struct iio_channels_mask *ch_mask = iio_get_channels_mask(iio_device_get_channels_count(dev));
//struct iio_channels_mask *ch_mask = iio_create_channels_mask(iio_device_get_channels_count(dev));
Copy link
Contributor

Choose a reason for hiding this comment

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

Still not addressed. Was it on purpose?

osc.c Outdated
else {
attribute = iio_device_find_attr(dev, attr);
ret = iio_attr_write_longlong(attribute, lval);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

fix indentation in here...

osc.c Outdated
} else if (debug) {
attribute = iio_device_find_debug_attr(dev, attr);
ret = iio_attr_write_longlong(attribute, lval);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

and here

osc.c Outdated
ret = iio_attr_write(attribute, value);
} else {
attribute = iio_device_find_attr(dev, attr);
ret = iio_attr_write(attribute,value);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

also odd in here though it seems it was wrong before

oscmain.c Outdated
@@ -255,7 +255,7 @@ gint main (int argc, char **argv)
if (!ctx)
connect_dialog(false);
// !!!!!!!
Copy link
Contributor

Choose a reason for hiding this comment

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

Is creating a default plot still problematic? If not, you should remove the comment with the exclamation marks.

@@ -2148,7 +2155,8 @@ static gboolean check_valid_setup_of_device(OscPlot *plot, const char *name)
gtk_widget_set_tooltip_text(priv->capture_button,
"Time Domain needs at least one channel");
return false;
} else if (dev && !dma_valid_selection(name, enabled_channels_mask | global_enabled_channels_mask(dev), nb_channels)) {
} else if (dev && !dma_valid_selection(name, enabled_channels_mask |
Copy link
Contributor

Choose a reason for hiding this comment

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

It's no longer necessary to check 'dev' since you added a check already which will exist this function.

Signed-off-by: Cristina Suteu <[email protected]>
use libiio1 stream object
use libiio1 API
re-enable plotting functions
TO DO: context clone and osc_test_value functions

Signed-off-by: Cristina Suteu <[email protected]>
cristina-suteu and others added 5 commits February 7, 2025 17:07
read attributes correctly when loading plugins
update osc_test_value function for both device and channel attrs
update osc_read_nonenclosed_value to do the same
remove context clone
remove unnecessary commented lines
fix indent issues

Signed-off-by: Cristina Suteu <[email protected]>
in osc.c, each struct iio_device and iio_channel get assigned some dynamically allocated memory which never gets freed.

Signed-off-by: Dan Nechita <[email protected]>
Copy link
Contributor

@dNechita dNechita left a comment

Choose a reason for hiding this comment

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

I am ok with this.

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.

3 participants