search

offa / plug

Software for Fender Mustang Amps.
GNU General Public License v3.0
54 stars 20 forks source link

Invalid amp id: 0 #1

Closed Wrzlprnft closed 5 years ago

Wrzlprnft commented 5 years ago 
~ » plug
QStandardPaths: XDG_RUNTIME_DIR not set, defaulting to '/tmp/runtime-wrzlprnft'
terminate called after throwing an instance of 'std::invalid_argument'
  what():  Invalid amp id: 0
[1]    1213 abort      plug

On Void Linux. Edit: The Amp is a Mustang 1 V.2. build passed the unittests. I checked the udev rules, the group exists and the user is part of it, and the amp seems to be recognized by the laptop.

~ » cat /proc/asound/cards
 0 [PCH            ]: HDA-Intel - HDA Intel PCH
                      HDA Intel PCH at 0xf7910000 irq 33
 1 [Amplifier      ]: USB-Audio - Mustang Amplifier
                      FMIC Mustang Amplifier at usb-0000:00:14.0-1, full speed

Did a preset factory reset on the amp, did not help. I don't know what to do for further troubleshooting. Is there a way to query the preset values on the amp? Edit: I only am at the place with the amp at weekends I haven't tried the "old" https://bitbucket.org/piorekf/plug/ on this particular installation yet.

Wrzlprnft commented 5 years ago

I just tried https://bitbucket.org/piorekf/plug/ and it starts and connects, and i used it to update the amp's firmware (to now 2.2). This repo's fork just has a quick flash of a window opening and closing on startup with the amp connected (edit: with the error reported above), or a "failed to open usb device" with the amp not connected.

offa commented 5 years ago

Thanks for reporting the issue. What version of Plug (= this fork) are you using? Does the problem occur always or is it working sometimes?

I experienced such a problem myself a while ago: At startup the application threw an exception because nothing (or garbage) was transmitted from the amp. After some retries I received the correct data. Unfortunately I couldn't figure out the problem yet.

I don't know what to do for further troubleshooting. Is there a way to query the preset values on the amp?

There's no query at the moment, but I can add a simple dump function to check the data from the amp.

Wrzlprnft commented 5 years ago

I cloned the git repo as is, so it's the master branch at the commit a52a583. Edit2: It also happens with the most recent release 1.4.0

Edit: so far the problem occurs every single time.

Edit3: cmake version: 3.15.3_1 qt5 version: 5.13.1_1 libusb-1.0.22_1

offa commented 5 years ago

Can you test with an older release (eg. 1.3.0)? But I assume all versions are affected.

Wrzlprnft commented 5 years ago

Release 1.3.0 seems to work every single time (when amp is off, it still starts but in disconnected state). Any hint to what commit i could start with in hunting the issue down?

Edit: Behaviour persists after reboot: 1.3 works, 1.4 and current HEAD don't.

offa commented 5 years ago

Unfortunately I can't reproduce it on my system. :-(

If there's a good version, it's possible to use git bisect to figure out what version introduced the error:

git bisect start
git bisect good v1.3.0
git bisect bad master

What mustang are you using btw.?

Wrzlprnft commented 5 years ago

Oh, i am always surprised when it comes to what git can do i either didn't know about, or did forget that that was a thing :D Ill try that.

A Mustang 1 V.2

offa commented 5 years ago

Thanks for your help! :+1:

Wrzlprnft commented 5 years ago

57f15eb44e16b38abcdc35078235782b3fb52c98 ends up to be the first bad commit. The one before i can start it and connect and load presets and do stuff.

I gladly help.

offa commented 5 years ago

This makes sense somehow: Previously an amp id of 0 was silently accepted, but since that commit it throws an error.

According enums "0" is not a valid amp id. The question is, does 0 represent a valid state or does the initial transmission fail do send anything useful? :confused:

Wrzlprnft commented 5 years ago

Now, my C and C++ programming is a bit rusty, and i have a couple of exams coming up the next two weeks. I doubt that i can come up with the code for dumping the first transmisson or what's saved on the amp in general. At least not in these two weeks. if you do, though, ill happily test it next weekend.

Edit: Or if there happens some sort of truncation fail on the transmission. i'd guess it's more the first transmission is nothing useful, though.

offa commented 5 years ago

I'm already on a very simple package dump :wink:.

Good luck for the exams btw.!

offa commented 5 years ago

Edit: Or if there happens some sort of truncation fail on the transmission. i'd guess it's more the first transmission is nothing useful, though.

From what I have experienced earlier, the first transmission sends either correct data (= no problem) or nothing / garbage (= exception).

offa commented 5 years ago

There's a very, very basic dump in the 176-invalid_amp_id branch. It simply writes the bytes of the initial transmission to stdout.

Wrzlprnft commented 5 years ago 
» ./plug
QStandardPaths: XDG_RUNTIME_DIR not set, defaulting to '/tmp/runtime-wrzlprnft'
---[Initial]------------------------
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
------------------------------------
terminate called after throwing an instance of 'std::invalid_argument'
  what():  Invalid amp id: 0
[1]    11193 abort      ./plug

Edit: and thanks!

offa commented 5 years ago

I see …

… at least it's not garbage …

offa commented 5 years ago

It's possible that the real data comes in another transmission. This worked in earlier releases (including the initial project) as those didn't check the values range.

The implementation isn't robust enough to handle malformed or invalid packages; the first transmission is evaluated whatever goes over the wire.

This definitely needs an improved communication which correctly reacts to the packages transmitted. In addition it shouldn't block the UI and the error handling needs some improvements too.

Unfortunately I can't come up with a fast fix here, as it's a major change in how things work.

Wrzlprnft commented 5 years ago

Oh, no hurry from my side, if i really need to use the software, i know to what commit i can revert to/what line of code i'd need to comment out (that would make it more unsafe, but be on my head for myself). Ill just keep an eye out for when/if you make changes in the future and test those. Or perhaps some day inspiration/motivation strikes me to work my way into the code and write something.

offa commented 5 years ago

I have implemented a first workaround to address this. Instead of crashing, an error message is shown in the UI and log. Still, there are major changes of the whole process necessary to really fix this issue.

The broken reconnect is fixed too now.

In some cases it may also indicate a missing amp support; I'm not sure if all amps of the V2 models are already integrated.

Please let me know if there are further problems.