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

Improve Linux part, implement functions in libsddc and fix debug stacktrace read in firmware #196

Closed
wants to merge 1 commit into from

Conversation

kpd200081
Copy link

I would use libsddc for RX888 mk2 on Linux, but facing that a lot of functions not implemented and streaming was not working for me.

There are still some bugs and place for improvements, but I decide to share my progress with others.

@ik1xpv
Copy link
Owner

ik1xpv commented Apr 18, 2021

Hi Pavel, many thanks for sharing your code progress. I think Linux support is very important, unfortunately my personal knowledge of Linux is low. Fortunately Franco, Howard, Hayati have directed the code evolution to go in the direction of trying to offer support to the Linux world as well.
Oscar

Core/fft_mt_r2iq.h Outdated Show resolved Hide resolved
libsddc/libsddc.cpp Show resolved Hide resolved
libsddc/libsddc.cpp Show resolved Hide resolved
libsddc/libsddc.cpp Outdated Show resolved Hide resolved
@howard0su
Copy link
Collaborator

appreciate your PR.

@kpd200081
Copy link
Author

Thanks for attention here, Howard and Oscar

@howard0su
Copy link
Collaborator

Thanks for attention here, Howard and Oscar

My pleasure. please cleanup and we can commit. I will also build a package for the libsddc for Linux/Windows for application to consume.

if(stream) {
// resume streaming
streaming_start(stream);
}
Copy link
Author

Choose a reason for hiding this comment

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

Here I was trying to avoid freezes when using device controls during active streaming. Now it freezes less, but not perfect, if anyone can suggest a better way to fix it I'll be glad.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I looked at libusb code once. I am not expert. It is my suspicion that we didn't handle multi thread well. I suggest this page:
http://libusb.sourceforge.net/api-1.0/libusb_mtasync.html

Copy link
Author

Choose a reason for hiding this comment

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

Oh yes, it looks like we have a situation described at the end of the article on your link. Thanks, I'll try to figure it out.

Copy link
Author

Choose a reason for hiding this comment

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

I tried to figure out what happens in bulk transfers with libusb, but with no success. I found that streaming only works if there is a control transfers to the device (such as reading a debug trace), but under such conditions, all other controls are delayed by about a second. Without the control transfers bulk transfers timeouts, it looks like it tries to work when setting the transfer timeout to lowest possible 1ms, but this is not enough for full functionality anyway.
My competence is not enough to resolve this issue.

Copy link
Author

Choose a reason for hiding this comment

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

Looks like I near solved this. Now it's possible to normally use streaming on linux, but sometimes streaming stops from libusb timeouts.
Also I was forced to reduce QUEUE_SIZE to 16 from 32, because with 32 I wasn't able get stable streaming with libusb.
Unfortunately, I can not test is something broke on Windows or not.

Copy link
Owner

Choose a reason for hiding this comment

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

Pavel thanks for the code. It compiles in windows. In release at first look it seems to run ok while in debug it stops streaming after start up.
We have to understand what's happening before merging it to master.
Howard in the while is making big change in #199 and #200.

Copy link
Author

@kpd200081 kpd200081 May 10, 2021

Choose a reason for hiding this comment

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

Thanks for testing Oskar.
Hm, it's looking as in debug we have enabled reading debug from fx3 and this not works...
It's strange that without my change in firmware, reading not working good on Linux...
Anyway debug read on linux currently not working because of timeouts in streaming so I will just revert this change.

Copy link
Author

Choose a reason for hiding this comment

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

I reverted changes in firmware, please check when you will have a time.

@kpd200081 kpd200081 force-pushed the libsddc-imp branch 4 times, most recently from 0e96ab3 to 74854ca Compare May 9, 2021 13:01
@kpd200081 kpd200081 marked this pull request as ready for review May 9, 2021 13:01
@kpd200081 kpd200081 force-pushed the libsddc-imp branch 3 times, most recently from 6620d9c to 39777f7 Compare May 10, 2021 10:06
Core/config.cpp Outdated
#include "config.h"

bool saveADCsamplesflag = false;
uint32_t adcnominalfreq = DEFAULT_ADC_FREQ;
uint32_t adcnominalfreq = DEFAULT_ADC_FREQ;
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we revert the changes in this file?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, didn't notice this unrelated change in diff - reverted.

Core/config.h Outdated
@@ -1,7 +1,7 @@
#ifndef _CONFIG_H_
#define _CONFIG_H_

#include "license.txt"
Copy link
Collaborator

Choose a reason for hiding this comment

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

all good changes but not related to linux usb. can we do these changes in a separate change?

Copy link
Author

Choose a reason for hiding this comment

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

Reverted all except QUEUE_SIZE change

{
return 0;
switch(t->samplerateidx)
Copy link
Collaborator

Choose a reason for hiding this comment

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

sample rate is related to adc rate.

Copy link
Author

Choose a reason for hiding this comment

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

Currently only two ADC rates supported in libsddc (64 and 128 MHz) and this relation handled here https://github.com/ik1xpv/ExtIO_sddc/blob/master/Core/RadioHandler.cpp#L154, isn't it?

@howard0su
Copy link
Collaborator

only suggestion is revert the non-related style change.

@kpd200081
Copy link
Author

Please do not merge this yet, I found problem in streaming stopping.

@kpd200081 kpd200081 marked this pull request as draft May 10, 2021 14:53
@kpd200081 kpd200081 closed this Jun 15, 2021
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

3 participants