General code style and conventions

[mgstingl]

Hi @beniroquai !
A couple of things I noticed which might improve maintainability and readability. Happy to further discuss any of these suggestions:

camelCasing vs. pythonic_casing

Firstly, an issue of casing in Python, common to most of the repository. As an example, in PositionerManager it would be more "Pythonic" to have no camelCase for properties and methods like forPositioning, which should look like:

    @property
    def for_positioning(self) -> bool:
        """ Whether the positioner is used for manual positioning. """
        return self.__for_positioning

It might make sense for the front-end, but it would be more consistent at least in the back-end.

Simpler initialization and default values

A nice "pythonic" one-liner for simplifying .json parsing and setting a default value that simplifies this:

        if positionerInfo.managerProperties.get('stepsizeX') is not None:
            self.homeSpeedX= positionerInfo.managerProperties['homeSpeedX']
        else:
            self.homeSpeedX= 15000

would look like:

        self.homeSpeedX= positionerInfo.managerProperties.get("homeSpeedX", 15000) # Checks for key "homeSpeedX" and if None return default value 15000.

Parsing the setup .json file more efficiently

Similar to this, in the managerProperties (example_uc2_labmaite.json) of the ESP32StageManager things could be simplified with dicts, which are more compatible to the position attribute, and then nicely parsed with the previous 1-liner:

{
  "positioners": {
    "ESP32Stage": {
      "managerName": "ESP32StageManager",
      "managerProperties": {
        "rs232device": "ESP32",
      "stepsize" = {"X":  0.3125,"Y":  0.3125,"Z": 0.3125},
      "home_speed" = {"X":  15000,"Y":  15000,"Z": 15000},
      "home_direction" = {"X":  -1,"Y":  -1,"Z": 1},
      "backlash":  ...
   }
}

Then, the initialization would look like:
self.homeSpeed = positionerInfo.managerProperties.get("homeSpeed", {axis: 15000 for axis in self.__axes })
Including the self.setupMotor, which could be called just once, taking advantage of the axes attribute:

def setupMotor(self, minPos, maxPos, stepSize, backlash, axis):
        for axis in self.__axes:
            self._motor.setup_motor(axis=axis, minPos=minPos[axis], maxPos=maxPos[axis], stepSize=stepSize[axis], backlash=backlash[axis])

Kind regards!

[beniroquai]

Hey @mgstingl thanks for writing such a detailed message. I think there are many things to be improved in the code and I would be insanely grateful if "someone" would do it :D

Hi @beniroquai ! A couple of things I noticed which might improve maintainability and readability. Happy to further discuss any of these suggestions:

camelCasing vs. pythonic_casing

Firstly, an issue of casing in Python, common to most of the repository. As an example, in PositionerManager it would be more "Pythonic" to have no camelCase for properties and methods like forPositioning, which should look like:

    @property
    def for_positioning(self) -> bool:
        """ Whether the positioner is used for manual positioning. """
        return self.__for_positioning

It might make sense for the front-end, but it would be more consistent at least in the back-end.

I totally agree. I must admit that I have never learned any programm and I learn things while doing. I think I have started with a pretty much random come-as-you-go order. Now I'm trying to stick to Pythonic.

Simpler initialization and default values

A nice "pythonic" one-liner for simplifying .json parsing and setting a default value that simplifies this:

        if positionerInfo.managerProperties.get('stepsizeX') is not None:
            self.homeSpeedX= positionerInfo.managerProperties['homeSpeedX']
        else:
            self.homeSpeedX= 15000

would look like:

        self.homeSpeedX= positionerInfo.managerProperties.get("homeSpeedX", 15000) # Checks for key "homeSpeedX" and if None return default value 15000.

A good to know. Sounds like a huge improvement! :)

Parsing the setup .json file more efficiently

Similar to this, in the managerProperties (example_uc2_labmaite.json) of the ESP32StageManager things could be simplified with dicts, which are more compatible to the position attribute, and then nicely parsed with the previous 1-liner:

{
  "positioners": {
    "ESP32Stage": {
      "managerName": "ESP32StageManager",
      "managerProperties": {
        "rs232device": "ESP32",
      "stepsize" = {"X":  0.3125,"Y":  0.3125,"Z": 0.3125},
      "home_speed" = {"X":  15000,"Y":  15000,"Z": 15000},
      "home_direction" = {"X":  -1,"Y":  -1,"Z": 1},
      "backlash":  ...
   }
}

Then, the initialization would look like: self.homeSpeed = positionerInfo.managerProperties.get("homeSpeed", {axis: 15000 for axis in self.__axes }) Including the self.setupMotor, which could be called just once, taking advantage of the axes attribute:

def setupMotor(self, minPos, maxPos, stepSize, backlash, axis):
        for axis in self.__axes:
            self._motor.setup_motor(axis=axis, minPos=minPos[axis], maxPos=maxPos[axis], stepSize=stepSize[axis], backlash=backlash[axis])

Kind regards!

Souds great! Here I was just sticking to what ImSwitch did originally. But I would be grateful if you can file a PR :)