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

Add functionality for MiniDSP 2x4HD #737

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

dennisfrett
Copy link
Contributor

@dennisfrett dennisfrett commented Sep 6, 2022

Additional functionality for the MiniDSP driver.

  • retrieve the current input source (Analog, Toslink or USB).
  • retrieve current configuration slot
  • setting volume / muted status

@Lauszus
Copy link
Collaborator

Lauszus commented Sep 7, 2022

Looking good. I've left one comment. Can you address this and then I'll merge it :)

@MarkLido
Copy link

MarkLido commented Sep 7, 2022

I added also the functionality to retrieve current config from MiniDSP 2x4HD.

@dennisfrett
Copy link
Contributor Author

I added also the functionality to retrieve current config from MiniDSP 2x4HD.

I can check if I can add these later this week. @Lauszus I don't know if you'd rather merge this change in already, or if you'd like to merge everything together later?

@MarkLido
Copy link

MarkLido commented Sep 7, 2022

Better later. After the function for "send" is added!?

@Lauszus
Copy link
Collaborator

Lauszus commented Sep 7, 2022

Okay great. Just let me know when it's ready to get merged :)

@dennisfrett dennisfrett changed the title Add functionality to retrieve input source from MiniDSP 2x4HD Add functionality for MiniDSP 2x4HD Sep 7, 2022
@dennisfrett
Copy link
Contributor Author

dennisfrett commented Sep 7, 2022

I updated the code to also retrieve the current config + I fixed the callback for input source not being called (Might be handy for you, @MarkLido ).

Updating volume has not been done yet.

However, @Lauszus , I'm having an issue, in OnInitSuccessful I do:

        RequestStatus();
        RequestInputSource();
        RequestConfig();

Which send commands, but don't listening for a response, so most of the time the response to the first command (request status), is just lost.

What would you suggest here?

I guess I'm using the library wrong, should I use ctrlReq instead? Or should I somehow busy wait for a response myself?

@MarkLido
Copy link

MarkLido commented Sep 7, 2022

Thanks @dennisfrett. The callback for input source works now. I added also the config callback in the ino. Works perfectly.

@dennisfrett
Copy link
Contributor Author

@MarkLido I've made a quick implementation for setting volume and muted status.

But I still think I might not be handling synchronous commands correctly, so any input from @Lauszus here would be awesome.

uint8_t SetMutedommand[] = {0x17, muted ? (uint8_t)0x01 : (uint8_t)0x00};

SendCommand(SetMutedommand, sizeof(SetMutedommand));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing new line.

return;
}

uint8_t SetVolumeCommand[] = {0x42, (uint8_t)(-2*volumeDB)};
Copy link
Collaborator

@Lauszus Lauszus Sep 9, 2022

Choose a reason for hiding this comment

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

Suggested change
uint8_t SetVolumeCommand[] = {0x42, (uint8_t)(-2*volumeDB)};
uint8_t SetVolumeCommand[] = {0x42, (uint8_t)round(-2.0f * volumeDB)};

void MiniDSP::setVolumeDB(float volumeDB) const {
// Only accept values between 0dB and -127.5dB.
// Don't do error handling.
if(volume > 0 || volume < -127.5){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if(volume > 0 || volume < -127.5){
if(volume < 0 || volume > -127.5) {

/**
* Send the "Request input source" command to the MiniDSP.
*/
void
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
void
void RequestInputSource() const;

/**
* Send the "Request config" command to the MiniDSP.
*/
void
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
void
void RequestConfig() const;

@Lauszus
Copy link
Collaborator

Lauszus commented Sep 9, 2022

I updated the code to also retrieve the current config + I fixed the callback for input source not being called (Might be handy for you, @MarkLido ).

Updating volume has not been done yet.

However, @Lauszus , I'm having an issue, in OnInitSuccessful I do:

        RequestStatus();
        RequestInputSource();
        RequestConfig();

Which send commands, but don't listening for a response, so most of the time the response to the first command (request status), is just lost.

What would you suggest here?

I guess I'm using the library wrong, should I use ctrlReq instead? Or should I somehow busy wait for a response myself?

An workaround would be to just add delay(1); in between the calls.

@dennisfrett
Copy link
Contributor Author

@Lauszus Writing this driver without a way too easily debug USB pakets seems quite difficult, and my current implementation has some bugs (delaying inbetween sending doesn't resolve the issue, setting volume only works the first time and not the second time, ...). To make my life easier I've purchased an Atmega32u4 in the hope I can set it up as a USB sniffer (https://github.com/matlo/serialusb).

Once I figure out what's going wrong and fix it, I'll update the PR.

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