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

Added C++ libxdf #13

Closed
wants to merge 1 commit into from
Closed

Added C++ libxdf #13

wants to merge 1 commit into from

Conversation

Yida-Lin
Copy link

Hi guys!

Sorry for the delay, here is the C++ XDF loader, that Clemens @cbrnr and I have been working on in the past year. It was specifically tailored for SigViewer so it may not align exactly as the MATLAB and Python loader, but overall it serves the purpose of loading and resampling XDF files well. Any advice on improvements would be appreciated.

Best,
Yida

@cboulay
Copy link
Collaborator

cboulay commented Mar 28, 2019

Hi @Yida-Lin ,

Discussion about libxdf has popped up again because we're worried about diverging matlab/python/labrecorder features. If we do end up making a libxdf then it would be nice if your library served as the starting point.

I've invited you at the xdf-modules organization. Would you be willing to make libxdf a separate repository and transferring it to that organization? You will still have full control over that repository.

I'm going to close this PR because it doesn't fit in this top-level repository, but it would be a very welcome addition to the xdf-modules organization.

@cboulay cboulay closed this Mar 28, 2019
@Yida-Lin
Copy link
Author

Hi @cboulay ,

Thanks for following up on this issue; I am more than happy to let the current libxdf serve as a starting point for the potential module.

The only problem I see is that, SigViewer (@cbrnr ) depends on the current version of libxdf, and if libxdf is modified, it may become incompatible with SigViewer. Or did I misunderstand that, you meant duplicating the libxdf repo first, then transferring it into the xdf repo?

Thanks

@cboulay
Copy link
Collaborator

cboulay commented Mar 28, 2019

It's better to not have duplicated code.

If you don't already have a libxdf repo, then I propose you create one in the xdf-modules group. From there we'll create a tag for its current state. SigViewer can link against this commit/tag.

Then we (you, me, @tstenner, anyone else interested) will modify the library so that it is useful in LabRecorder, the Python importer, and the Matlab importer. Hopefully it won't be too much work to also update SigViewer to be able to use this new version of the library, but SigViewer can continue to use the old version until that happens.

How does that sound?

@cboulay
Copy link
Collaborator

cboulay commented Mar 28, 2019

@Yida-Lin
Copy link
Author

@cboulay Sounds great, I have just finished transferring.

@cboulay
Copy link
Collaborator

cboulay commented Mar 28, 2019

By the way, along similar lines, if you want then we can also put SigVisualizer in the github.com/labstreaminglayer organization. Again, you'll have full control over it. I recommend it to people all of the time so it would be good if it were more visible, and maybe others will collaborate on it to help iron out the bugs when there are multiple streams.
Let me know!

@Yida-Lin
Copy link
Author

@cboulay I agree, I never had enough time resource to finish SigVisualizer so it would be great if there are more hands to work on it. Sounds good to me.

@cboulay
Copy link
Collaborator

cboulay commented Mar 28, 2019

For the smoothest transition with SigVisualizer, I think first you need to add me as an admin to the repo so I can transfer its ownership to labstreaminglayer group, then I will add you back as an admin to the SigVisualizer repo.

I'm on my way out the door right now so I won't be able to do this for about 4 or 5 hours.

@Yida-Lin
Copy link
Author

Yida-Lin commented Mar 28, 2019

I didn't see a way to add admin in Settings (probably because I am using a free account).

Alternatively, you could add me to the labstreaminglayer group, and I will transfer it just like libxdf.

@cboulay
Copy link
Collaborator

cboulay commented Mar 29, 2019

Sorry I meant add as a collaborator. https://help.github.com/en/articles/inviting-collaborators-to-a-personal-repository
Once added you can change my permissions between Read, Write, and Admin. That last one should give me power to transfer the repo.

Unlike the xdf org, for the labstreaminglayer org we are trying to keep the organization-level members down to the same core group. Individual repos within the organization can be admin'd by outsiders.

If I can't transfer the repo then I'll add you temporarily, probably tomorrow.

@Yida-Lin
Copy link
Author

@cboulay I see, great. I thought you are already a collaborator at https://github.com/Yida-Lin/SigVisualizer
so hopefully it already works.

@cboulay
Copy link
Collaborator

cboulay commented Mar 29, 2019

Ah, I see that collaborators can't transfer user repositories, only the owner can. OK, then can you transfer the repository to me first, then I will transfer it to labstreaminglayer?
https://help.github.com/en/articles/transferring-a-repository#transferring-a-repository-owned-by-your-personal-account

@Yida-Lin
Copy link
Author

You may need to delete your forked repo before I can transfer

@Yida-Lin
Copy link
Author

image

@cboulay
Copy link
Collaborator

cboulay commented Mar 29, 2019

Sorry! Done.

@Yida-Lin
Copy link
Author

@cboulay Cool, done here as well.

@cboulay
Copy link
Collaborator

cboulay commented Mar 29, 2019

https://github.com/labstreaminglayer/App-SigVisualizer
Now we need someone to go through the sccn/labstreaminglayer Wiki and create alternate Python-only tutorials to supplement the Matlab tutorials, but that's for another day.

@Yida-Lin
Copy link
Author

@cboulay Sounds great! Could you please share a link to the Matlab tutorials?

@cboulay
Copy link
Collaborator

cboulay commented Mar 29, 2019

They're just in the Wiki. See the menubar on the right here: https://github.com/sccn/labstreaminglayer/wiki

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