Skip to content

Conversation

@wmww
Copy link
Contributor

@wmww wmww commented Jun 21, 2025

Split EVENT_DEVICE_ADDED into EVENT_ZCM1_ADDED and EVENT_ZCM2_ADDED so the proper product ID can be used, this removes two FIXMEs and makes it work on Linux (NOT tested on OSX/windows, no code changes were needed on windows, and the OSX code changes were relatively simple)

Split EVENT_DEVICE_ADDED into EVENT_ZCM1_ADDED and EVENT_ZCM2_ADDED so
the proper product ID can be used, this removes two FIXMEs and makes it
work on Linux (NOT tested on OSX/windows, no code changes were needed on
windows, and the OSX code changes were relatively simple)
Copy link
Owner

@thp thp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solves the problem, but would it be nicer if the make_event() / event_callback() function / the on_monitor_update_pair() / on_monitor_update_moved() functions (the monitor callback) just gets an additional unsigned short pid as parameter?

Also this looks weird (first checking for both events, then changing something based on the event; clearly the handling of both events is similar enough):

if (event == EVENT_ZCM1_ADDED || event == EVENT_ZCM2_ADDED) {
   [...]
   unsigned short pid = event == EVENT_ZCM1_ADDED ? PSMOVE_PID : PSMOVE_PS4_PID 

Adding the enums like this breaks ABI already (which would be fixed by ordering it like EVENT_ZCM1_ADDED , EVENT_DEVICE_REMOVED, EVENT_ZCM2_ADDED, leaving the existing values intact), so might as well break the ABI in a way that makes things clearer (while still allowing people to ignore the PID if they so chose).

If the unsigned short pid just gets passed as a parameter, the moved.cpp change would just nicely clean itself up without any event changes (and just the removal of the local unsigned short pid variable, which is now added as parameter), and the psmovepair.c code would - apart from the added unused pid parameter - stay the same.

Alternatively, instead of using unsigned short pid, you could also use enum PSMove_Model_Type model, and map from/to the PID in the monitor code, and map back from it in moved.cpp? Since moved_monitor is already quite low level, it's probably file if we pass the pid there, since we already have path and serial there as well.

tl;dr:

  1. Encode different models as additional unsigned short pid parameter
  2. Leave enum MonitorEvent untouched
  3. Update the code for the added callback paramter

@wmww
Copy link
Contributor Author

wmww commented Jul 14, 2025

Sorry for the delay, hows that?

Copy link
Owner

@thp thp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@thp thp merged commit 45d5f34 into thp:master Jul 14, 2025
0 of 4 checks passed
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.

2 participants