basic mdxf support #350
basic mdxf support #350
Conversation
| return false; | ||
| } | ||
|
|
||
| mdxf_device = aciodrv_device_open_path(config_mdxf.port, config_mdxf.baud); |
There was a problem hiding this comment.
question: Does it make sense to have the baud rate configurable? You mentioned that the current implementation uses the full serial bandwidth at 115200 baud.
There was a problem hiding this comment.
Yes and/or no, I can go either way on that.
To me settings for a com port location should be where and what baud, just inherently to give the user the flexibility in case they have an odd setup.
For example MDXF does reply and work at 57600, but it really doesn't make sense to use it at anything but the fastest supported 115200.
So I could go either way.
There was a problem hiding this comment.
Ah, I see. I personally prefer to expose as less configuration as required as this limits complexity regarding potential error/odd combinations. I leave this one up to you to decide if it's worth exposing that. But, I believe it's still worth raising the complexity point.
| uint32_t pad = 0; | ||
|
|
||
| // pull the state of the p4io & get the state of the mdxfs | ||
| if (p4io_ctx) { |
There was a problem hiding this comment.
readability suggestion: Have one guard at the beginning and short-curcuit with a return 0;. Reduces the nesting level a bit and makes the rest of the flow easier to read, imo.
There was a problem hiding this comment.
I mean this is more of a code style more than anything, I'm personally not a fan of short circuiting code as it leads to multiple "exits" of a function that can be hard to trace.
But for added context one of the reasons I wrote the code this way was I only have physically the two MDXF boards in possession, so by checking for the p4io context and allowing the rest of the functions (the serial MDXF coms) to run, I could troubleshoot more before I transfer the binary over to the cabinet.
There was a problem hiding this comment.
Sure, we can leave as is as it doesn't change behaviour.
| p4iodrv_cmd_coinstock(p4io_ctx, (uint8_t *) &coin_buff.raw); | ||
| p4iodrv_cmd_portout(p4io_ctx, (uint8_t *) &light_buff.raw); |
There was a problem hiding this comment.
general remark/stuff we need to discuss: All those setter functions should not interact with the hardware in a blocking manner (at least that's how it was intended afair). Only ddr_io_read_pad for reading inputs from the p3io jamma part and ddr_io_set_lights_extio for communicating with the extio. But, this contribution also highlights the short comings of the interface as it is modeled after the p3io + extio hardware setup which is different from the pure p4io one.
Therefore, we need to discuss (and then document properly) which functions are supposed to do what that users of the ddrio API are using them correctly.
I think it's fair to include these changes in general, but maybe label them somehow as "experimental/beta" as you also pointed out that it needs more work (maybe log a warning on init saying "experimental mdxf support!" or something like that?).
Open to your thoughs and suggestions about this.
There was a problem hiding this comment.
Yeah this is certainly more of an area of discussion in terms of the *io API in general.
I wrote this more of a "I want to play older games on my p4io/mdxf setup" so I wrote additional things (like the cycling colors) to make an SD lighting output of a game more vibrant and stimulate the RGB lighting on the cabinet.
But that does conflict with the concept of if you are writing a game that does want to specifically set the lights of a cabinet to a specific color, the API does not allow you to do this action. A new function call would need to be added to allow this (ex: setting the header/neon RGB).
But adding new functions can break compatibility over time as in this codebase newer functions are statically loaded instead of checking for the function's existence at runtime. This is mainly an issue with applying the DLLs in other games/contexts, but it is something to consider as a whole.
Ex; trying to use an older ddrio dll fork with a newer tool, would cause the tool to crash because it assumes that a newer function exists when it doesn't.
Another question to be talked about would be should it be up the *io dll to "translate" lights? Or just be solely a way for a game/shim to explicitly set the lights in question?
Or is it some combination of both (like for example how I checked to see if a caller was sending HD menu lights, then defaulted to using those over the SD menu lights, etc).
There was a problem hiding this comment.
I wrote this more of a "I want to play older games on my p4io/mdxf setup" so I wrote additional things (like the cycling colors) to make an SD lighting output of a game more vibrant and stimulate the RGB lighting on the cabinet.
But that does conflict with the concept of if you are writing a game that does want to specifically set the lights of a cabinet to a specific color, the API does not allow you to do this action. A new function call would need to be added to allow this (ex: setting the header/neon RGB).
Yeah, that's fine and I am aware of the limitations of the current API.
But adding new functions can break compatibility over time as in this codebase newer functions are statically loaded instead of checking for the function's existence at runtime. This is mainly an issue with applying the DLLs in other games/contexts, but it is something to consider as a whole.
Yeah, it's something the project had on its radar for quite a while. However, it requires major changes and reworking of the bemanitools backend to support proper DLL loading and API versioning. I got a branch up that is labeled "bemanitools 6" which includes such changes. I just haven't found the time and energy to continue that.
Therefore, the best way forward is working with the current limitations and crafting reasonable solutions/workarounds as long as these don't break API compatibility for now, or do something dumb.
Another question to be talked about would be should it be up the *io dll to "translate" lights? Or just be solely a way for a game/shim to explicitly set the lights in question?
Well, I guess that falls under "work with the limitations we got for now"?
Or is it some combination of both (like for example how I checked to see if a caller was sending HD menu lights, then defaulted to using those over the SD menu lights, etc).
Maybe instead of making this an implicit choice based on data/logic flow, how about exposing this as a configuration option (with a sane default)?
There was a problem hiding this comment.
Overall, changes are fine to be merged with the following requests:
- We need to talk about the ddrio interface and how it's expected to be used with the different IO implementations we got now. That needs some clarification and alignment to ensure we don't confuse ourselves and anyone using the ddrio library implementations
- Label the current state of the implementation (somewhat) visibly as "experimental" to manage expectations
Will await your responses @dinsfire64 as you might have some more thoughts/ideas before we merge this.
2 participants
adds basic polling to mdxf devices, which were shipped starting with the "white" hd ddr cabinets, as well as the "white" hd cab ddrio (p4io for cabinet, mdxf for foot inputs).
preforms the following tasks:
AC_IO_CMD_MDXF_POLLddrio-p4ioallows for gameplay with any ddrio toolset, controls lights and input.ddrio-p4iosupports mapping of SD lights to HD by cycling RGB colors like the main game doesddrio-p4iosupports config for the mdxf com port, defaults to COM2/115200ddriotestfor the neons, as it did not work correctly (it never checkedstateonly the return of scanf).ddriotestthis is the first step of the process, which is manually polling the mdxf board for the switch state. this is inherently very slow (approx 2ms rtt per side), so the device supports an "autopolling" mode.
currently i want to ensure this pr stands on its own before working with others to engineer a solution to ingesting the autopoll. these devices will saturate a 115200 baud rate link with the current state of both sides, causing buffers to overflow or a core being dedicated to just emptying the buffer of mostly useless data.
so i need more insight from others to the best way to engineer in windows a ring buffer or some sort of event system such that the state of the mdxf devices are only "sampled" at 1ms intervals, while still taking in the 115200 data stream correctly.
for now i want to evaluate this basic version as it sets the state for further development.