Turnouts.cpp: Turnout::load is redefining servo angles.

[FrightRisk]

David Morse suggests line 108 should be

if (data.tStatus & STATUS_PWM) tt=create(data.id,data.tStatus & STATUS_PWMPIN, data.inactiveAngle+data.moveAngle,data.inactiveAngle );

[Neil-McK]

There are various problems with the current Turnout servo code, which make it produce strange results. This is one of them. And the parameters xxxAngle are not angles, they're PWM parameters in the range 0-4095 for 0-100% pulse width. Also, the moveAngle parameter is limited to one byte (-128 to +127), so anything more or less than this will be calculated modulo 256, with undesirable results. A standard SG90 servo has the extreme values of around 100 to 480 for a full 180 degree sweep, i.e. to accommodate the full range would require -380 to +380. I've experimented with various ways to overcome this, but none were satisfactory, and would be making a bad job worse. The Turnouts, PWM, Servo, and I2C functionality have been completely revised in the neil-hal experimental branch which hopefully will be incorporated into the master branch in due course.

[Neil-McK]

I've created a branch turnout-bugfix to cover this. I've modified the parameter names to remove the reference to angle (now activePosition and inactivePosition), and altered the struct layout so that the two parameters are stored in 12-bit fields, allowing both to be in the range 0-4095 (as in the neil-hal branch). This overcomes the restrictions above, and allows servo position, or LED brightness, to be controlled through the PCA9685. While there, I also added missing null pointer checks in case anyone is near the limit of RAM, and parameter validation to avoid overflow of the fields in the struct.

I'm not sure whether David Morse was actually using the code or if it was just an observation. There is no command for creating a Servo turnout in the current version of the CS software. They can be created from mySetup.h, but then they wouldn't need to be saved to EEPROM.

[Neil-McK]

I've just remembered why I didn't do this change previously. Will change it back.

[FrightRisk]

Is there a way to just have degrees? There can only be 360 of them. Then map all the devices to that. You are already ahead of me since I was going to ask about 10 or 12 bit data. I guess every byte counts if we had 100 servos and had to add another byte.

What are you having to change back? The 12+12?

[Neil-McK]

I had originally toyed with changing the usage of the fields within the structure to either scale the 'moveAngle' by 4 to fit (allowing a range of +/- 511), or to store the full PWM positions (0-4095 for both ends). The issue that @Asbelos raised was that this could cause problems for anyone who already has a servo turnout set up in EEPROM, since the PWM could try to move the servo outside of its normal range after the restart, with potential damage to the servo or devices (turnouts etc) connected to it. Therefore I was holding out until the neil-hal implementation, where we would delete any existing EEPROM contents on the first start of the new software, for safety.

Since there currently no command for creating servo turnouts, I don't think it's likely that this would happen, but you can guarantee that if it isn't considered, someone will encounter a problem. David Morse, for example, appears to be using servo turnouts.

I'm disinclined to use degrees for the parameter units, since different servo types will move by different angles when presented with the same control pulses. And even servos of the same type from the same manufacturer can exhibit different characteristics. For example, a spec for the Tower SG90 (http://www.ee.ic.ac.uk/pcheung/teaching/DE1_EE/stores/sg90_datasheet.pdf) specifies that the range for -90 to +90 degrees is a pulse length of 1.0 to 2.0 ms (period 20ms). So this is 5-10% pulse width, or 204-409 in terms of the PWM 'ticks' parameter to the PCA9685. I have two SG90 servos set up on my desk. If I send the commands 204-409, expecting a 180 degree sweep, they both move by less than 90 degrees. One of them moves by nearly 180 degrees if I send 104-480 as parameters, and the other moves by about 145 degrees. So consistently moving a servo by 90 degrees when the user specifies a move angle of 90 degrees is impossible, without building individual servo calibrations into the software.

The parameter we can control fairly consistently is the pulse length, as a percentage (0-100%, with servos operating in the range around 3-13%), microseconds (0-20000, with servos operating in the range around 500-2500), or PWM ticks (0-4095 with servos operating in the range around 100-500). Then the user can determine what parameters place their particular servo in the desired positions and build these parameters into the turnout definition. If they change the servo, they should change the turnout definition accordingly.

For servo (PWM) positioning parameter units, we could use percent, (one byte storage, low resolution, only 100 steps, hardware independent), microseconds (2 bytes storage, high resolution, hardware independent) or PWM ticks (1.5 bytes storage, optimal for 12-bit PWM resolution). I've opted for PWM ticks in neil-hal, partly because that's what's in the current CS software. I think that's probably the best compromise.

If the range of the positioning parameters is restricted to that used by MG90 servos and equivalents, it will restrict the ability to use the PWM outputs other purposes, e.g. linear motors, LEDs etc. By allowing the pulse range to be controlled over its full range, we will provide higher flexibility. I don't see any value in building in unnecessary restrictions which may require modification in the future.

[Neil-McK]

I was going to revert to the 12+8 allocation of inactivePosition and moveAngle, with validation of the moveAngle to ensure that it doesn't exceed the available space. I will retain the null reference fixes.

[FrightRisk]

If someone is upgrading to EX from Classic or a previous version, can we not just issue a warning that they will have to erase their EEPROM and re-enter their turnouts? Or we provide a little program that reads the output of their <s> command or whatever and then put all that into something that can be uploaded and saved to their EEPROM the first time?

My question is if we were developing a new product and it could be however we wanted, how would we design it? I get no one likes breaking changes. But we have one chance to get it right and tell people what the upgrade path is. Some will grumble, but some of those will see the benefits of a new method and go through a bit of inconvenience. So what does not compromising look like? And if we do (by going with ticks) what do we lose? Obviously a compromise is not always a bad thing.