search

SasaKaranovic / VU-Server

VU Dials Server Application
https://vudials.com
26 stars 12 forks source link

potential bug in dial_easing_get_config() #17

Closed feeph closed 7 months ago

feeph commented 8 months ago

The function signature for this function is dial_easing_get_config(self, dialID) but it doesn't seem like the parameter dialID is actually used.

https://github.com/SasaKaranovic/VU-Server/blob/ee237b6d6842b43d927727743737e846b1415b53/dial_driver.py#L300

The serial command does not use the provided value: https://github.com/SasaKaranovic/VU-Server/blob/ee237b6d6842b43d927727743737e846b1415b53/dial_driver.py#L303

The existence of 

dial_easing_dial_step(self, dialID, value)
dial_easing_dial_period(self, dialID, value)
dial_easing_backlight_step(self, dialID, value)
dial_easing_backlight_period(self, dialID, value)

indicates the easing config is a dial-specific setting and the bug is most likely the usage of 'COMM_DATA_NONE'. The function should probably use COMM_DATA_SINGLE_VALUE instead.

ret = self._sendCommand(self.commands.COMM_CMD_GET_EASING_CONFIG, self.data_type.COMM_DATA_SINGLE_VALUE, 1, dialID)
feeph commented 8 months ago

Tested it out. Confirmed.

Using COMM_DATA_SINGLE_VALUE and providing the dial's index works as expected. I'm able to fetch dial-specific easing settings.

SasaKaranovic commented 7 months ago

Hi feeph, thank you for reporting this.

This is actually an issue and not an issue at the same time. Initially dial easing was supposed to be stored on the dials. However during development it was decided that dial easing should be stored and cached on the VU Server side. So this function should never be used, and even if used it would report the value that was previously sent by the VU Server.

With that said, I agree that function signature should be updated for posterity and potential future use. There is a PR #26 that should address this issue.