+mizapf Posted July 5, 2022 Author Share Posted July 5, 2022 LOG_WARN should be output by default when you use "-log" or "-oslog". You can add more debug flags in the source file. The websocket client, however, cannot output via the MAME logging facility, as it is an external library; you have to add printf lines. Edit: The location where I suspect the issue is in the file https://github.com/mamedev/mame/blob/master/src/lib/util/client_ws.hpp, line 304, in the "handshake()" method. The client_ws is actually not part of a third-party library but of MAME, and could probably be fixed more easily if required. More detail: The problem might arise after the asio::async_read_until(..."\r\n\r\n"), which reads up to the blank line after the header of the HTTP response. Quote Link to comment Share on other sites More sharing options...
PeteE Posted July 6, 2022 Share Posted July 6, 2022 (edited) Wow. You weren't kidding about "lots of asynchronous handling with callbacks and lambda expressions." I've never before experienced a stack trace in GDB taking minutes to print while deciphering the function names due to the sheer complexity of the templates in the Boost asio library. I wonder if we're seeing different symptoms. The handshake looks fine, and the received ASYNC message is parsed correctly. The breakdown as far as I can tell happens immediately after the ASYNC here in client_ws.hpp. //Close connection if masked message from server (protocol error) if(first_bytes[1]>=128) { const std::string reason("message from server masked"); auto kept_connection=connection; send_close(1002, reason, [](const std::error_code& /*ec*/) {}); if(on_close) on_close(1002, reason); return; } The first_bytes usually contains {0xff, 0x82 or 0xff} so it triggers the above close on the websocket. From the traffic on WireShark, there is no masked message. I'm having trouble figuring out where this data is actually read from... I'm also very suspicious about this function in class Message in client_ws.hpp: std::string string() const { std::stringstream ss; ss << rdbuf(); return ss.str(); } If I print the message string in tipi_card_device::websocket_incoming() case 1: LOGMASKED(LOG_WARN, "Got text message: size=%zu %s\n", message->size(), message->string()); Sometimes it will show extra garbage control chars at the end of the ASYNC string... something's not right. Edited July 6, 2022 by PeteE 2 Quote Link to comment Share on other sites More sharing options...
+mizapf Posted July 6, 2022 Author Share Posted July 6, 2022 There is one issue with the logging: The websocket runs on a separate thread, and the logging is not thread-safe. That is, it may mess up the log by seemingly unprintable characters (I guess it is only NUL characters, caused by different locations of the file pointer in the threads). Maybe you should replace the logging output by printf. As I said, it took me many hours to track down the issue as far as I described in the posting above. Quote Link to comment Share on other sites More sharing options...
PeteE Posted July 7, 2022 Share Posted July 7, 2022 (edited) In client_ws.hpp, there seems to be a possibility that there could be leftover data in the message->streambuf before the asio::async_read is called, and the asio::transfer_exactly doesn't account for that. I've tried changing the lines asio::async_read(*connection->socket, message->streambuf, asio::transfer_exactly(2), to: asio::async_read(*connection->socket, message->streambuf, asio::transfer_exactly(std::max(2 - message->streambuf.size(), std::size_t{0})), And this helps in some cases, but not all. For example, after a console reset, it gets stuck on the ASYNC string websocket packet, even though there is enough data (7 bytes) already in the message->streambuf.size(). Maybe it doesn't like transfer_exactly(0)? It works sometimes. Also, need to copy leftover data from one message to the next: //Next message std::shared_ptr<Message> next_message(new Message()); //Copy any leftover data in the streambuf into the next message next_message->streambuf.commit(buffer_copy( next_message->streambuf.prepare(message->streambuf.size()), message->streambuf.data())); read_message(next_message); If you need an actual patch, let me know. Also! Disable or remove this, otherwise it will recurse endlessly when there are already enough bytes in message->streambuf: if(bytes_transferred==0) { //TODO: This might happen on server at least, might also happen here read_message(message); return; } Edited July 7, 2022 by PeteE 1 Quote Link to comment Share on other sites More sharing options...
PeteE Posted July 9, 2022 Share Posted July 9, 2022 (edited) Well, the above code doesn't work, but my hypothesis was correct. Using std:max to calculate the number of bytes didn't work because std::size_t is unsigned, so it would end up requesting a negative-converted-to-unsigned number of bytes, and wait forever. I also noticed that reading message->string() multiple times would consume the stringbuf every time, so the next call could return an empty string or part of the next message on the socket. My fixes are here: https://github.com/mamedev/mame/compare/master...peberlein:mame:master Edit: You could also remove the Transfer Mode configuration option now. The server sends "ASYNC" if it supports that mode, otherwise the client should keep sending syncs when appropriate. Edited July 9, 2022 by PeteE 3 1 Quote Link to comment Share on other sites More sharing options...
+mizapf Posted July 9, 2022 Author Share Posted July 9, 2022 Thank you for your work! As it seems, it works so far for me, but I did not fully understand why. How does it solve the problem that the ASYNC is already in the streambuf, but the async_read in the read_message never got invoked because there was no reaction from the epoll_wait? Quote Link to comment Share on other sites More sharing options...
globeron Posted July 9, 2022 Share Posted July 9, 2022 (edited) On 6/16/2022 at 6:45 AM, globeron said: it needs the .rpk format (currently I almost converted most .bin to .rpk format, very straight forward and edit the .xml file) to be used with Batocera (uses Mame for the TI and works better than the TI/SIM on the retropie) (but I need some time to add the images and sample videos for Batocera then will share it). @mizapf are you able to run these .rpks in MAME )? (they do work in js99er.net when I load the rpk's) superfly.rpk honey_hunt.rpk TerryTurtle.rpk (it does start in JS99, but indicates it requires a MBX console) (similarly Champion Baseball does the same, but that one starts in MAME / Batocera until the screen that an MBX console is needed) ImHiding.rpk (this gives a bluescreen, also in JS99er.net, and same when selecting it from the js99er.net software list) Edited July 9, 2022 by globeron Quote Link to comment Share on other sites More sharing options...
PeteE Posted July 9, 2022 Share Posted July 9, 2022 (edited) 5 hours ago, mizapf said: Thank you for your work! As it seems, it works so far for me, but I did not fully understand why. How does it solve the problem that the ASYNC is already in the streambuf, but the async_read in the read_message never got invoked because there was no reaction from the epoll_wait? My guess is that the async_read doesn't look at how much data is already in the streambuf. So even though it had 7 ASYNC bytes in it already after reading the websocket headers, it would epoll_wait until 2 more bytes arrived. That's why I wrote the message.transfer function which will return how many bytes are needed to fill streambuf to the amount needed, and if the streambuf already has that many, it will return 0, allowing the async_read to immediately call the lambda to process the existing data. The biggest clue was printing out the value of message->streambuf.size() before each async_read. Glad it fixed it for you too. Will you be able to merge in my changes? If there's anything else I need to do, let me know. Edited July 9, 2022 by PeteE 1 Quote Link to comment Share on other sites More sharing options...
+mizapf Posted July 9, 2022 Author Share Posted July 9, 2022 The Superfly RPK is not working (see -oslog, lots of invalid memory accesses), but the superfly.zip (see WHTech). Interestingly, the zip version has a ROM dump and a GROM dump. Also, the ZIP versions of Honey Hunt, Terry Turtle, and I'm Hiding are working. Remember that the ZIPs contain the original ROM dumps as far as I could see. 1 Quote Link to comment Share on other sites More sharing options...
+mizapf Posted July 9, 2022 Author Share Posted July 9, 2022 @PeteEI did some research during the last days as well. It seemed to me that the handshake() would have worked if the async_read (called by read_message) did not wait for new data but just processed what is left in the queue. OK, and if you can make async_read leave the epoll_wait trap by declaring 0 bytes to be read, this explains why it works. We should find a better name for "transfer"; the name is misleading. I'd expect some function that moves data by that name and not one that returns a value. When I read asio::transfer_exactly(message->transfer(2)), how should one call that part in parentheses? Probably needed_for(int n)? Or maybe the transfer method returns the complement, i.e. transfer1(n) = n-transfer(n). So transfer1 could be called available. One good news: I think we are the only people who include the client_ws in MAME, so there is no problem with breaking other drivers. Quote Link to comment Share on other sites More sharing options...
PeteE Posted July 9, 2022 Share Posted July 9, 2022 5 minutes ago, mizapf said: We should find a better name for "transfer"; the name is misleading. I'd expect some function that moves data by that name and not one that returns a value. When I read asio::transfer_exactly(message->transfer(2)), how should one call that part in parentheses? Probably needed_for(int n)? Or maybe the transfer method returns the complement, i.e. transfer1(n) = n-transfer(n). So transfer1 could be called available. One good news: I think we are the only people who include the client_ws in MAME, so there is no problem with breaking other drivers. I had difficulty deciding on the name. I like "needed_for". Quote Link to comment Share on other sites More sharing options...
+mizapf Posted July 9, 2022 Author Share Posted July 9, 2022 OK, I just pushed the client_ws.hpp to the master branch. I'll check the tipi implementation once more, already removed the transfer mode setting. Edit: Also pushed the new tipi.{h,cpp}. I'll build new binaries and put them on WHTech. 1 Quote Link to comment Share on other sites More sharing options...
+mizapf Posted July 10, 2022 Author Share Posted July 10, 2022 I uploaded the new builds to WHTech for Windows, Linux, and Raspbian as "mame20220710b_*". If you like you can download them and unpack them as usual over your current installation. This release contains the fix for the TIPI with real Raspberry Pi. 3 1 Quote Link to comment Share on other sites More sharing options...
globeron Posted July 10, 2022 Share Posted July 10, 2022 18 hours ago, mizapf said: The Superfly RPK is not working (see -oslog, lots of invalid memory accesses), but the superfly.zip (see WHTech). Interestingly, the zip version has a ROM dump and a GROM dump. Also, the ZIP versions of Honey Hunt, Terry Turtle, and I'm Hiding are working. Remember that the ZIPs contain the original ROM dumps as far as I could see. @mizapf can you help how to start the .zip files in MAME ? cd \mame mame64.exe ti99_4a -cart c:\mame\carts\superfly.zip -ioport peb -ioport:peb:slot2 32kmem -ioport:peb:slot3 speech -ioport:peb:slot8 h cd \mame mame64.exe ti99_4a -gromport multi -cart c:\mame\carts\superfly.zip -ioport peb -ioport:peb:slot2 32kmem -ioport:peb:slot3 speech -ioport:peb:slot8 h probably not as there are more grom files cd \mame mame64.exe ti99_4a -gromport single -cart c:\mame\carts\superfly.zip -ioport peb -ioport:peb:slot2 32kmem -ioport:peb:slot3 speech -ioport:peb:slot8 h Quote Link to comment Share on other sites More sharing options...
+mizapf Posted July 10, 2022 Author Share Posted July 10, 2022 The ZIP files are not referenced as files (like the RPKs). They are pre-registered in the software list (hash/ti99_cart.xml) and are referenced by name. The ZIP file is normally named as <cartridgename>.zip. That is, you have to use the plain file name without extension. The ti99_cart.xml is essentially a collection of the former layout.xml, but by another schema. mame ti99_4a -cart exbasic mame ti99_4a -cart editass mame ti99_4a -cart superfly mame ti99_4a -cart honeyhnt If you misspell it, you get a list of possible corrections. The ZIP files must be located in a path specified in the ROMpath. Quote Link to comment Share on other sites More sharing options...
globeron Posted July 10, 2022 Share Posted July 10, 2022 1 hour ago, mizapf said: The ZIP files are not referenced as files (like the RPKs). They are pre-registered in the software list (hash/ti99_cart.xml) and are referenced by name. The ZIP file is normally named as <cartridgename>.zip. That is, you have to use the plain file name without extension. The ti99_cart.xml is essentially a collection of the former layout.xml, but by another schema. mame ti99_4a -cart exbasic mame ti99_4a -cart editass mame ti99_4a -cart superfly mame ti99_4a -cart honeyhnt If you misspell it, you get a list of possible corrections. The ZIP files must be located in a path specified in the ROMpath. Thank you. I will try it (it seems .zip files do not work in Batocera, because of the file structure setup), but I prefer the .rpk format for Batocera setup to keep it standardized. these .rpk also do not work but they work in js99er.net tetris.rpk spad_xiii.rpk compu-car.rpk Quote Link to comment Share on other sites More sharing options...
+mizapf Posted July 10, 2022 Author Share Posted July 10, 2022 22 minutes ago, globeron said: Thank you. I will try it (it seems .zip files do not work in Batocera, because of the file structure setup), but I prefer the .rpk format for Batocera setup to keep it standardized. The ZIPs are the standard for MAME (and the RPKs are the exception), but I guess you mean standard with respect to our emulator environments. If Batocera relies on MAME, it should be able to use the ZIPs, as the ZIP loader is a core function of MAME (used for the system ROM ZIPs as well, like ti99_4x.zip). Sounds like a configuration issue to me. Edit: If RPKs don't work, try to use "gromemu" instead of "standard" as the PCB type (layout.xml). The "standard" type only accepts 6K GROMs (real GROMs), not 8K (which is why I call it "GROM Emulation"). 1 Quote Link to comment Share on other sites More sharing options...
globeron Posted July 11, 2022 Share Posted July 11, 2022 9 hours ago, mizapf said: The ZIPs are the standard for MAME (and the RPKs are the exception), but I guess you mean standard with respect to our emulator environments. If Batocera relies on MAME, it should be able to use the ZIPs, as the ZIP loader is a core function of MAME (used for the system ROM ZIPs as well, like ti99_4x.zip). Sounds like a configuration issue to me. Edit: If RPKs don't work, try to use "gromemu" instead of "standard" as the PCB type (layout.xml). The "standard" type only accepts 6K GROMs (real GROMs), not 8K (which is why I call it "GROM Emulation"). Thank you, I will try to play around with the different configurations, some others I got to work last time, but these RPKs seem to give a bit trouble ? (and I am trying to understand why JS99er.net does not have the issue loading these .rpk ?) Quote Link to comment Share on other sites More sharing options...
+9640News Posted July 11, 2022 Share Posted July 11, 2022 @mizapf My limited testing, things are looking good. I did try to run my AfterHours BBS code under the GPL environment. Need to check some things out as I was having some troubles and not sure the cause yet. Beery Quote Link to comment Share on other sites More sharing options...
Asmusr Posted July 11, 2022 Share Posted July 11, 2022 6 hours ago, globeron said: and I am trying to understand why JS99er.net does not have the issue loading these .rpk ? JS99er is probably more relaxed about the different PCB types, e.g. it doesn't distinguish between original and emulated GROMs. You can't use JS99er to validate that RPKs are correctly configured. 3 Quote Link to comment Share on other sites More sharing options...
+mizapf Posted July 11, 2022 Author Share Posted July 11, 2022 Yes, the RPKs were designed at a time when the GROMs were not yet emulated at circuit level in MAME, and when we had full GROM space dumps. The ZIP cartridges, however, have per-circuit dumps (6K), and to avoid confusion ("standard" would have meant different things for RPK and for ZIP), I restricted the standard type to 6K GROMs, and added the "gromemu" as the type which allows 8K per GROM. The technical background is that there are cartridges pretending to have GROMs, but actually use an EPROM and an address counter, thus emulating GROMs. 1 Quote Link to comment Share on other sites More sharing options...
+jedimatt42 Posted July 16, 2022 Share Posted July 16, 2022 On 7/9/2022 at 5:33 PM, mizapf said: I uploaded the new builds to WHTech for Windows, Linux, and Raspbian as "mame20220710b_*". If you like you can download them and unpack them as usual over your current installation. This release contains the fix for the TIPI with real Raspberry Pi. Thank you @mizapf and @PeteE! Finally I can debug and experiment with the TIPI ROM without pulling chips while in the comfort of the same PI I use on real hardware. 6 Quote Link to comment Share on other sites More sharing options...
+jedimatt42 Posted July 16, 2022 Share Posted July 16, 2022 Except for the >1800 crubase makes me sad. Maybe more mame users are Geneve users than 4A users. <port tag=":ioport:peb:slot7:tipi:SW1" type="DIPSWITCH" mask="31" defvalue="24" value="16" /> How does 24 map to >1800 and 16 map to >1000 ? oh, it is the decimal MSByte of the crubase... That's surprising, as the hardware only has a 4 bit jumper block. And super surprising that I can't find a general mame command line argument for setting something so formally identifiable as the named dipswitch... but I guess I can script edits to the file in cfg/. I commonly want to test >1000 and >1100 and never care about >1800... I'm sure this is nothing that 'sed' can't handle. Quote Link to comment Share on other sites More sharing options...
+mizapf Posted July 16, 2022 Author Share Posted July 16, 2022 In those cases, I suggest to use a special config directory. Use "-cfg_directory xxx" and apply the settings in the MAME menu; they will be stored in xxx/ti99_4a.cfg. The problem is that the current GeneveOS requires the TIPI to be set to >1800, and if you forget it, it simply does not work at all and gives you no hint about the problem. Even I forgot to set the CRU base back to >1800 after doing some other work with the Geneve and wondered what I could have broken in the meantime, until I remembered to set the address back to >1800. 1 Quote Link to comment Share on other sites More sharing options...
+jedimatt42 Posted July 16, 2022 Share Posted July 16, 2022 1 hour ago, mizapf said: In those cases, I suggest to use a special config directory. Use "-cfg_directory xxx" and apply the settings in the MAME menu; they will be stored in xxx/ti99_4a.cfg. The problem is that the current GeneveOS requires the TIPI to be set to >1800, and if you forget it, it simply does not work at all and gives you no hint about the problem. Even I forgot to set the CRU base back to >1800 after doing some other work with the Geneve and wondered what I could have broken in the meantime, until I remembered to set the address back to >1800. The DSK mapping of TIPI used in a 4A won't work for people either if they include an actual floppy controller in the system as is typical. And it too just won't work and gives no hint about the problem. This is about authors preference, and that is OK. js99er.net doesn't even let you set the TIPI crubase. So even though the crubase emulation for TIPI is not true to the hardware in MAME, it is welcomely flexible. #!/bin/bash TIPI_CRU=${TIPI_CRU:-1000} SW1=`echo "ibase=16; ${TIPI_CRU%00}" | bc` sed -i "/tipi:SW1/s/ value=\"..\"/ value=\"${SW1}\"/g" ./cfg/ti99_4a.cfg ./mame ... $@ With the above scripting I can easily invoke my 4a mame script as: TIPI_CRU=1100 ./tipi.sh -cart ~/dev/github/jedimatt42/fcmd/FCMD.RPK to change the default. Or I can skip including the environment override and the script will set it to my preferred default, without having to get back into the UI. 1 Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.