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 data exchange through a global shared state block #303

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vbousquet
Copy link
Contributor

@vbousquet vbousquet commented Aug 24, 2024

This commits allows to share previously unshared data without overloading too much the API by adding a single data block for pulsed state, device state, modulated segments, multiple DMDs, raw DMD frames (for correct rendering and allow coloring) and video.

This commit contains the skeleton and base implementation to set up the ground for more testing on client side (VPX) when support will be more advanced there. It also gives a ground to implement in LibPinMame.

The aims of these changes are for VPX but could be useful for other client as well:

  • allow VPX to perform correct DMD rendering (needs unprocessed DMD frames),
  • allow VPX's plugin to perform coloring (needs unprocessed DMD frames),
  • allow VPX to include a backglass renderer with support for fading lamps & flashers (could be done with existing interface but at the price of lots of data exchange and likely some API bloat),
  • allow VPX to support modulated segments,
  • allow VPX to add support for video output (MrGame machines, ...).

[Edit: multiple force-push to fix stupid build bugs (missing min/max, ...)]

@toxieainc
Copy link
Member

Hi! In principle this looks good, but as i don't have time to dig into details at the moment, is this somehow future-proof, e.g. could hypothetically newer platforms like SPIKE, etc be supported, too, without breaking backwards compatibility?
(e.g. if somebody would run an older VPX version with a then even newer VPM version)

@vbousquet vbousquet force-pushed the vpm_getstate branch 2 times, most recently from c382cc0 to 7d90da7 Compare August 24, 2024 19:56
@vbousquet
Copy link
Contributor Author

It is supposed to be as future proof as possible (and especially for Spike) but it's difficult to be fully sure and in some situation, this needs client to be updated:

  • the client must agree with the server on the data format, so the first field is a version id : if it does not match then no further processing should be done. This means that when PinMame will change its state block format, then it will change this version id and all client will have to be updated (copy/paste the declaration, recompile, adjust code),
  • all fields come with their data size (or struct size) to allow processing (or skipping) of future fields by today implementation,
  • instead of DMD, it supports generic 'display' which can be a DMD / LED matrix / RGB display. Regarding Spike, I would say that the main difference would be the video which would benefit from this display data span (exchange the LCD as an RGB array), but I can't tell if the performance will be good enough (Spike has an HD screen so datacopy are somewhat expensive),
  • output are modeled as 'devices' with a data stride of the device state included. This should allow to support devices that could need more state information without breaking anything (it would be nice to model at least solenoids and stepper motors).

What I had in mind was to propose this change (eventually keeping it in a separated branch), then move VPX forward (multiple display for DMD & Backglass, and a skeleton for plugin are already comited but this needs far more love to really give some useful feature). It will likely lead to identify some defect in the proposed design/data format that will need some updates here.

@toxieainc
Copy link
Member

Sounds good. I mainly wonder about this, as i guess that there will be a longer transition phase for VPX to a potential VP11, similar (most likely even longer) to what happened with VP9 and VPX.
So people will run both versions, BUT will need a current VPM, too. But that may also be possible to be solved by still doing updates to VPX then.

@vbousquet vbousquet force-pushed the vpm_getstate branch 6 times, most recently from b77790c to 94b9c9b Compare September 3, 2024 21:48
@vbousquet vbousquet force-pushed the vpm_getstate branch 10 times, most recently from 41981ed to 1881c59 Compare September 14, 2024 15:36
@vbousquet vbousquet force-pushed the vpm_getstate branch 3 times, most recently from d2d327d to c3cb903 Compare September 18, 2024 20:53
@vbousquet vbousquet force-pushed the vpm_getstate branch 2 times, most recently from 5963f74 to 22ff1fb Compare September 28, 2024 08:17
@vbousquet vbousquet force-pushed the vpm_getstate branch 4 times, most recently from f3476f9 to a81ce81 Compare October 13, 2024 19:50
@toxieainc
Copy link
Member

@vbousquet How should we progress with this one?

@vbousquet vbousquet marked this pull request as draft October 17, 2024 20:55
@vbousquet
Copy link
Contributor Author

@vbousquet How should we progress with this one?

I have converted this PR to a draft since I think it is not good enough for merging. I will continue to push the integration on other parts to validate the design before converting it back to a normal PR. Indeed, the last weeks have shown that implementing all the uses may show API change needs and I think it would be better to merge it only after these have stabilized.

For the time being, DMD data exchange is quite nice. Next to be tackled would be alphanumeric displays, then displays (sRGB screens which still needs real use case with a VPX bridge to be validated), then lamps, then GPIO (solenoids / motors / ...). Until then, the API change risk is too high.

If you prefer, this can be closed for now.

@toxieainc
Copy link
Member

As you wish, whatever is easier for you. Maybe some (incl. me ;)) want to follow your progress.

@vbousquet vbousquet force-pushed the vpm_getstate branch 6 times, most recently from 89b381b to 91ab576 Compare November 2, 2024 11:16
@freezy
Copy link
Member

freezy commented Nov 18, 2024

Subscribed ;)

@vbousquet vbousquet force-pushed the vpm_getstate branch 2 times, most recently from bb67041 to 22a5337 Compare November 21, 2024 22:19
@vbousquet vbousquet force-pushed the vpm_getstate branch 3 times, most recently from 99acca8 to 764cd94 Compare December 11, 2024 17:54
@vbousquet vbousquet force-pushed the vpm_getstate branch 7 times, most recently from 5512819 to 9fe616b Compare December 26, 2024 18:39
This commits allows to share prviously unshared data without overloading too much the API by adding a single data block for pulsed state, device state, modulated segments, multiple DMDs, raw DMD frames (for correct rendering and allow coloring) and video.

This commit contains the skeleton and base implementation to set up the ground for more testing on client side  (VPX) when support will be more advanced there. It also gives a ground to implement in LibPinMame.
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