probonopd / MiniDexed

Dexed FM synthesizer similar to 8x DX7 (TX816/TX802) running on a bare metal Raspberry Pi (without a Linux kernel or operating system)
https://github.com/probonopd/MiniDexed/wiki
1.13k stars 81 forks source link

Add network support by omersiar #747

Open probonopd opened 2 weeks ago

probonopd commented 2 weeks ago

image

Continuation from https://github.com/probonopd/MiniDexed/pull/744

This adds wired and wireless network for supported models:

Closes https://github.com/probonopd/MiniDexed/issues/43 Thanks @omersiar

This allows us to send MIDI e.g., from DAWs, Dexed on the PC, or even sysex for voices from sites like patches.fm

Documentation: https://github.com/probonopd/MiniDexed/wiki/Networking#ethernet-and-wlan-support

github-actions[bot] commented 1 week ago

Build for testing: MiniDexed_2024-11-14-e9373ce Use at your own risk.

probonopd commented 1 week ago

MiniDexed_2024-11-14-e9373ce still has the stuck notes issue. Possibly because we are using a too new circle?

After reverting the changes applied to main branch, resolving ftp compiler warnings and handling wireless disconnections properly, I can say that my setup is now stable.

Which circle commit/branch are you using @omersiar?

probonopd commented 1 week ago

Has anyone tested wireless on a RPi Zero 2 W so far?

Banana71 commented 1 week ago

Build for testing: MiniDexed_2024-11-14-e9373ce Has anyone tested wireless on a RPi Zero 2 W so far?

Networking doesn't work on my RPi Zero 2 W in gadget mode. After the version number, TG1 > is displayed and the Rapsberry crashes.

omersiar commented 1 week ago

I can reproduce stuck notes with Step48 but not with Step47.

757 disables mdnsresponder and handles wireless drops more properly with unreliable wireless setups

Probably @probonopd you need to revert circle mdns patch as well. Done it in PR

github-actions[bot] commented 1 week ago

Build for testing: MiniDexed_2024-11-15-8a67e3c Use at your own risk.

probonopd commented 1 week ago

This one seems to work for me on a RPi Zero 2 W đź‘Ť

Let's test the heck out of this!

Update 1: It's been playing MIDI over WLAN for over 90 minutes now... without getting stuck

diyelectromusic commented 1 week ago

I'll just leave this here (from https://opguides.info/music/midi/)

"Over Network

Look, I’m not going to say you can’t do wireless or networked midi. You definitely can. It’s just that there’s alway something that goes wrong. The ’normal’ way to do it is using RTP MIDI. This should work cross platform:

Sometimes this works 100% perfectly, other times it’s a fucking nightmare. Best of luck."

Kevin

omersiar commented 1 week ago

Mine is also working straight for 2 hours

Sometimes this works 100% perfectly, other times it’s a fucking nightmare. Best of luck." Kevin

Can confirm. But it is great in MiniDexed context, which makes much easier to interact with it via browser or other tools.

Banana71 commented 1 week ago

RPi4: rtpMIDI connects quickly and works perfectly. FTP works. The connection is terminated after about 20 seconds without a transfer, but can be reconnected immediately.

github-actions[bot] commented 1 week ago

Build for testing: MiniDexed_2024-11-16-4972d30 Use at your own risk.

probonopd commented 1 week ago

Check the new Wiki page https://github.com/probonopd/MiniDexed/wiki/Networking - improvements welcome.

github-actions[bot] commented 1 week ago

Build for testing: MiniDexed_2024-11-16-f41bfc0 Use at your own risk.

github-actions[bot] commented 1 week ago

Build for testing: MiniDexed_2024-11-16-d7e950d Use at your own risk.

probonopd commented 1 week ago

Wired network also tested positively on a Raspberry Pi 3.

probonopd commented 1 week ago

Edit: Also works with Ethernet UDP networking together with the MIDI Button Navigation functionality to simulate keys being pressed on the MiniDexed device (which is neat for e.g., test automation):

https://github.com/probonopd/MiniDexed/wiki/Networking#udp-midi

github-actions[bot] commented 1 week ago

Build for testing: MiniDexed_2024-11-16-973008d Use at your own risk.

probonopd commented 1 week ago

@omersiar works with dns-sd with no stuck notes. Great workđź‘Ť

github-actions[bot] commented 6 days ago

Build for testing: MiniDexed_2024-11-16-ef69318 Use at your own risk.

probonopd commented 6 days ago

Now multiple MiniDexed devices can be distinguished from each other by setting different hostnames.

github-actions[bot] commented 6 days ago

Build for testing: MiniDexed_2024-11-16-784a280 Use at your own risk.

probonopd commented 6 days ago

MiniDexed_2024-11-16-7198c9d still shows wpa_supplicant.conf and allows it to be downloaded.

github-actions[bot] commented 6 days ago

Build for testing: MiniDexed_2024-11-16-286b7c8 Use at your own risk.

github-actions[bot] commented 6 days ago

Build for testing: MiniDexed_2024-11-16-bfca612 Use at your own risk.

probonopd commented 6 days ago

@omersiar do you have an idea how to block wpa_supplicant.conf from being downloaded? My attempts are not working so far.

github-actions[bot] commented 6 days ago

Build for testing: MiniDexed_2024-11-17-8b2e670 Use at your own risk.

probonopd commented 6 days ago

Thanks to @rsta2 there is now syslog logging over the network. It works for me when I hardcode the IP address like this:

            static const u8 SysLogServer[]   = {192, 168, 0, 143}; // FIXME: Don't hardcode this, use m_INetworkSyslogServerIPAddress instead
            static const u16 usServerPort    = 8514;        // standard port is 514
            CIPAddress ServerIP (SysLogServer);
            CString IPString;
            ServerIP.Format (&IPString);
            LOGNOTE ( "Sending log messages to %s:%u",
            (const char *) IPString, (unsigned) usServerPort);

            new CSysLogDaemon (m_pNet, ServerIP, usServerPort);

I can see all logging messages arriving at the syslog server on my host computer.

It even seems to buffer log messages that are created early in the boot process:

Syslog server listening on 192.168.0.143:8514
Press any key to exit...0:00:00.000 >1 - 192.168.0.60 uibuttons - - - MIDI Button on msg: 47 (2f)
00:00:00.048 >1 - 192.168.0.60 uibuttons - - - MIDI Button on msg: 48 (30)
00:00:00.083 >1 - 192.168.0.60 uibuttons - - - MIDI Button on msg: 49 (31)
00:00:00.117 >1 - 192.168.0.60 uibuttons - - - MIDI Button on msg: 50 (32)
00:00:00.151 >1 - 192.168.0.60 uibuttons - - - MIDI Button on msg: 51 (33)
00:00:00.184 >1 - 192.168.0.60 uibuttons - - - MIDI Button on msg: 52 (34)
00:00:00.220 >1 - 192.168.0.60 uibuttons - - - MIDI Button on msg: 53 (35)
00:00:00.235 >1 - 192.168.0.60 uibuttons - - - MIDI Button on msg: 54 (36)
00:00:00.270 >1 - 192.168.0.60 ui - - - MIDI Button channel set to: OMNI
00:00:00.303 >1 - 192.168.0.60 ui - - - Button User Interface initialized
00:00:00.337 >1 - 192.168.0.60 ui - - - Rotary encoder initialized
00:00:00.388 >1 - 192.168.0.60 syxfile - - - Banks successfully loaded #0
00:00:00.550 >1 - 192.168.0.60 syxfile - - - 19 Banks loaded. Highest Bank loaded: #27
00:00:00.550 >1 - 192.168.0.60 minidexed - - - Serial MIDI interface enabled
00:00:00.550 >1 - 192.168.0.60 minidexed - - - Program Change: Enabled for Voices
00:00:00.552 >1 - 192.168.0.60 Performance - - - Number of Performance Banks: 1 (last = 1)
00:00:00.560 >1 - 192.168.0.60 Performance - - - Loaded Default Performance Bank - Last Performance: 128
00:00:00.561 >1 - 192.168.0.60 mcore - - - CPU core 1 started
00:00:00.561 >1 - 192.168.0.60 mcore - - - CPU core 2 started
00:00:00.561 >1 - 192.168.0.60 mcore - - - CPU core 3 started
00:00:00.561 >1 - 192.168.0.60 minidexed - - - Initializing WLAN
00:00:00.561 >1 - 192.168.0.60 wlan - - - ether4330: chip 43430 rev 2 type 1
00:00:00.561 >1 - 192.168.0.60 wlan - - - ether4330: firmware ready
00:00:00.562 >1 - 192.168.0.60 wlan - - - ether4330: addr ...
00:00:00.562 >1 - 192.168.0.60 minidexed - - - wlan and wpasupplicant initialized
00:00:00.562 >1 - 192.168.0.60 wpa - - - Setting country code to 'DE'
00:00:00.562 >1 - 192.168.0.60 wpa - - - Trying to associate with ... (SSID='...' freq=2462 MHz)
00:00:00.562 >1 - 192.168.0.60 dhcp - - - No response from server. Retrying.
00:00:00.562 >1 - 192.168.0.60 wpa - - - Associated with ...
00:00:00.562 >1 - 192.168.0.60 wpa - - - WPA: Key negotiation completed with ... [PTK=CCMP GTK=CCMP]
00:00:00.562 >1 - 192.168.0.60 wpa - - - CTRL-EVENT-CONNECTED - Connection to ... completed (auth) [id=0 id_str=]
00:00:00.562 >1 - 192.168.0.60 dhcp - - - No response from server. Retrying.
00:00:00.563 >1 - 192.168.0.60 dhcp - - - IP address is 192.168.0.60
00:00:00.563 >1 - 192.168.0.60 minidexed - - - Sending log messages to 192.168.0.143:8514
00:00:00.563 >1 - 192.168.0.60 udpmididevice - - - RTP Listener initialized
00:00:00.563 >1 - 192.168.0.60 udpmididevice - - - UDP MIDI receiver initialized
00:00:00.563 >1 - 192.168.0.60 minidexed - - - FTP daemon initialized
00:00:00.563 >1 - 192.168.0.60 ftpd - - - Listener task spawned
00:00:00.563 >1 - 192.168.0.60 ftpd - - - Listener: waiting for connection
00:00:00.563 >1 - 192.168.0.60 mdnspub - - - Publish service dtdx
00:00:00.563 >1 - 192.168.0.60 mdnspub - - - Publish service dtdx

Amazing! đź‘Ť

But this does not work (with NetworkSyslogServerIPAddress=192.168.0.143 in minidexed.ini):

            CIPAddress ServerIP = m_pConfig->GetNetworkSyslogServerIPAddress().Get();
            static const u16 usServerPort    = 8514;        // standard port is 514
            CString IPString;
            ServerIP.Format (&IPString);
            LOGNOTE ( "Sending log messages to %s:%u",
            (const char *) IPString, (unsigned) usServerPort);

            new CSysLogDaemon (m_pNet, ServerIP, usServerPort);

What am I doing wrong? How can I get rid of the hardcoded IP address? (While we are at it, we should also check whether the IP address is 0 and if it is, skip the whole syslog thing.)

omersiar commented 6 days ago

The type for syslog SysLogServer is not matching what you define in config. It is not IPAddress. it is char string so m_Properties.GetString would work.

omersiar commented 6 days ago

@omersiar do you have an idea how to block wpa_supplicant.conf from being downloaded? My attempts are not working so far.

Yes, found a dirty way

ftpworker.cpp.patch

Command: RETR wpa_supplicant.conf Response: 553 Reading this file is not allowed

github-actions[bot] commented 6 days ago

Build for testing: MiniDexed_2024-11-17-631fa53 Use at your own risk.

probonopd commented 6 days ago

@omersiar seems like I had to do

https://github.com/probonopd/MiniDexed/blob/46ccb9ca12ae45bf60a93e0cfe6d4eb063fdfe28/src/config.cpp#L213-L217

as advised here. I wonder if we would need to do the same thing to the lines directly above it.

probonopd commented 6 days ago

I'm pretty happy with this PR overall now.

In the log I see very frequent

06:31:23.512 >1 - 192.168.0.60 icmp - - - Time exceeded (0)
06:31:23.513 >1 - 192.168.0.60 icmp - - - Time exceeded (0)
06:31:24.502 >1 - 192.168.0.60 mdnspub - - - Send failed
06:31:31.512 >1 - 192.168.0.60 icmp - - - Time exceeded (0)
06:31:31.516 >1 - 192.168.0.60 icmp - - - Time exceeded (0)
06:31:32.507 >1 - 192.168.0.60 mdnspub - - - Send failed
06:31:39.517 >1 - 192.168.0.60 icmp - - - Time exceeded (0)
06:31:39.520 >1 - 192.168.0.60 icmp - - - Time exceeded (0)
06:31:40.520 >1 - 192.168.0.60 mdnspub - - - Send failed

The advertised service is recognized properly by the host computer.

Is this expected @omersiar?

omersiar commented 6 days ago

It should be the other way around, if the syslog class expects a string then config should return a string.

github-actions[bot] commented 6 days ago

Build for testing: MiniDexed_2024-11-17-96285ab Use at your own risk.

probonopd commented 6 days ago

Well, I'm not going to touch syslog anymore since it's working now :)

omersiar commented 6 days ago

I'm pretty happy with this PR overall now.

In the log I see very frequent

06:31:23.512 >1 - 192.168.0.60 icmp - - - Time exceeded (0)
06:31:23.513 >1 - 192.168.0.60 icmp - - - Time exceeded (0)
06:31:24.502 >1 - 192.168.0.60 mdnspub - - - Send failed
06:31:31.512 >1 - 192.168.0.60 icmp - - - Time exceeded (0)
06:31:31.516 >1 - 192.168.0.60 icmp - - - Time exceeded (0)
06:31:32.507 >1 - 192.168.0.60 mdnspub - - - Send failed
06:31:39.517 >1 - 192.168.0.60 icmp - - - Time exceeded (0)
06:31:39.520 >1 - 192.168.0.60 icmp - - - Time exceeded (0)
06:31:40.520 >1 - 192.168.0.60 mdnspub - - - Send failed

The advertised service is recognized properly by the host computer.

Is this expected @omersiar?

This is due if network connection is not reliable, don't you get also the network disconnected messages?

If that is not the case then network stack thinks it is connected but packages may not go through.

probonopd commented 6 days ago

Thing is, it's playing MIDI coming in over rtpMIDI nicely, and I can upload firmware no problem. No network disconnected messages either. So for me it's purely cosmetic. This is on a Raspberry Pi Zero 2 W.

Maybe we could increase some timeouts and/or TTL?

probonopd commented 5 days ago

@diyelectromusic please review this if you have a chance. I'd like to get it merged before master deviates too much. Alone the possibility to output logs somewhere else than HDMI proves really useful. Thanks!

github-actions[bot] commented 5 days ago

Build for testing: MiniDexed_2024-11-17-b3e528a Use at your own risk.

diyelectromusic commented 5 days ago

@diyelectromusic please review this if you have a chance.

As I mentioned in the other PR there is far too much code for me to comment on the implications for the MiniDexed codebase.

I'm afraid I'm unable to advise on any of this change other than repeat my general concern - adding a range of scheduled tasks to handle network activity will, by definition, interfere with the audio digital signal processing at some point. I just don't know if it will be significant or not, especially if other Pi versions are factored in.

How has the networking scheduling scheme been designed (and why)? How will it fit alongside the main ProcessSound loop and architecture of MiniDexed as it stands? Has anyone looked at the process latency this introduces now? If FTPing files over, what does run-time updating of the SD card do to the rest of the processing threads? How will network timeouts be handled and will they suspend processing in odd places? How is concurrency between menu updates and network updates handled (and how do we ensure there are no race-conditions or consistency problems)? And so on...

I've not seen any discussion of how the networking stack has been architected into the existing code. And I don't know enough about the architecture of MT32-Pi to know if a straight port of the same approach works here.

I've feel like I've managed to get to grips with the core codebase (pretty much), but there is no way I can try to get inside this code, so if you plan on accepting it, I think you need to think very carefully about who is going to maintain it and debug it (and the rest of the system now having to cope with it of course) if there are issues further down the line.

Kevin

probonopd commented 5 days ago

Yes, I understand your concerns @diyelectromusic. I view this whole networking code as an opt-in feature that should, if switched off, have no impact. So that whenever an issue comes up, we can compare with networking switched off. I've tested this quite a bit over the last days. And no, I don't advise on uploading files over FTP during a live performance ;-)

github-actions[bot] commented 5 days ago

Build for testing: MiniDexed_2024-11-17-f881791 Use at your own risk.

diyelectromusic commented 5 days ago

Ok, so it would appear (according to the Scheduler documentation in Circle) that the non-pre-emptive, cooperative multitasking scheduler will only ever work on core 0 in a multi-core system...

So in that case, these networking tasks cannot interfere with the audio DSP side of things which all happen on cores 1,2 and 3.

Any potential issues will be in things like UI handling and handling of non-networked MIDI, so it would be good to know if the USB and serial MIDI handling are affected by the networking; and then how the UI functions whilst the networking is running.

Aside: I think this call to ->Yield() probably shouldn't be there - this is in the non-multicore (so Pi V1) area of code:

void CMiniDexed::Process (bool bPlugAndPlayUpdated)
{
    CScheduler* const pScheduler = CScheduler::Get();
#ifndef ARM_ALLOW_MULTI_CORE
    ProcessSound ();
    pScheduler->Yield();
#endif

I'm also not clear why there are so many calls to Yield in the CMiniDexed::Process() - e.g. in every part of the performance file handling. Was this to fix some issues that have been found with the scheduling?

I really think this needs some developer documentation adding to the wiki to explain how the CTask and CScheduler facilities of circle are being used to support the networking.

I'd also like to see more checks/comments in the networking code to make it clear what is called when and to make sure things will drop out if things aren't properly initialised. There seems to be a few assumptions about order and call sequences to ensure that the networking isn't enabled when not configured. Should any of this sequencing change in the future (I don't know why it would, but things do shuffle around with time) then those assumptions would break.

Another Aside: I think printf() was used in mididevice.cpp as LOGNOTE doesn't always work in all contexts - e.g. if called from a serial port interrupt or something like that - I forget exactly... (although I'm not sure those printfs are particularly useful in there anymore anyway...)

I'm not sure I quite follow the logic in CMiniDexed::UpdateNetwork() - is that just performing the network startup? If so, why is it called for every scan of Process()? I'm not following the interplay between bNetIsRunning, m_bNetworkInit, and m_bNetworkReady - can we make this a proper state machine if these are states the system passes through - and are these states appearing on separate calls to UpdateNetwork() or are they all expected to happen within the same call?

Are those LOGPANIC calls really the most appropriate response to a networking issue? Also, response to various other bits of the networking failing seem inconsistent (there is no checking at all for m_UDPMIDI.Initialize() for example).

btw - should we be saying "applemidi" - wouldn't it be better to say rtpmidi... (and we'd not be building someone else's company name into the source of an open project...)?

There seem to be some build updates as part of these changes that don't seem specific to this change?

I've mentioned before that I'm really not a fan of on-the-fly patching of major source files. If you really want to build the git hash into the firmware, can we put it in its own include file or something so we're not trying to patch userinterface.cpp? Wouldn't need a s// then either - just echo it out to the file. (isn't "." a wildcard anyway? So wouldn't this actually patch every occurrence of the string "Loading" and any following three characters no matter what they are? Which could even include a newline and the start of the next line iirc...)

As I say, I've not been able to follow the networking code itself, I've just tried to think through any ramifications for the core MiniDexed functionality...

Kevin

soyersoyer commented 5 days ago

If we already have wifi, ftpd, syslog, I think we should also have sshd :P Has anyone tried to use 8x synth_dexed on Linux? I think it could be interesting with PREEMPT_RT.

probonopd commented 5 days ago

I don't think synth_dexed has been ported to Linux. Might be a fun experiment for someone, though. If it is done in a way that keeps dependencies minimal (e.g., only ALSA, no jackd) it might run on a very stripped down "embedded Linux" type system containing not much more than the kernel, but allow for easier implementation of things like USB gadget mode as a sound device, MIDI device, and at the same time USB host for MIDI controllers, mass storage, etc. (I think) and networking, including ssh. Let me know if you ever start experiments in that direction. But that'd be a completely new development unrelated to today's MiniDexed. When I started MiniDexed I wanted something small and efficient, and not gigabytes of software with thousands of files.

github-actions[bot] commented 4 days ago

Build for testing: MiniDexed_2024-11-19-5aad2ef Use at your own risk.