Adding support for 16 channel PWM expansion to libraries

[rimai]

The other way is to check for presence of the module like this:

Code: Select all

Wire.beginTransmission(I2CPWM);
present = Wire.endTransmission();
if (present==0)
{
  //Do something, we got 8bit
}
else
{
  //Do something, we got 16bit
}

[AlanM]

Right, I'm doing that.

One weirdness is that the 8bit values are actually interpreted everywhere as percents, and the int values will be absolute int's from either 0 to 255 or 0 to 4095, not percents. I think I have it worked out.

I'm overloading everything between bytes and ints and trying to make the old versions that used bytes still work so no one gets broken code if they don't change anything because they assumed those were percentages, but if someone decides to use the new stuff and uses ints we interpret that as either 0-255 or 0-4095 raw values, not percents. Does that seem reasonable?

But then I run into the problem that the ExpansionChannel[] array is a byte array, so I create a new array called ExpansionChannelInts and have to start checking in the GetChannelValue function to see which one has values in it and return the right one because I can't overload the GetChannelValue function to produce both int and byte outputs. Can only overload with arguments, heh.

[AlanM]

The other way is to check for presence of the module like this:

Code: Select all

Wire.beginTransmission(I2CPWM);
present = Wire.endTransmission();
if (present==0)
{
  //Do something, we got 8bit
}
else
{
  //Do something, we got 16bit
}

Cool, I see that you do that with your Present variable now. Not sure where that var is read, but it makes sense.

[AlanM]

I'm trying hard to make it not change functionally at all if someone doesn't change their code as one constraint, but I'm also trying to get more resolution out of the PWM pins. Instead of the 100 levels of dimming which seems totally fine for pumps it would be nice to have 255 or 4096 across the board for light. Just overloading the PWM class functions for different arguments types isn't getting me there. It seems like passing in a constant from the main .ino file when making the call is by default putting in an int which the runtime casts to a byte when the function is executed, so the byte version of the overloaded function never gets called anyway. I could have totally different functions that I'd name as "HiRes" or something, but it seems silly to do that.

The pump profiles use the DCPump classes and assume you're putting in percentages from 0 to 100, and they call the PWM classes and feed in numbers from 0 to 100 to those classes to adjust the pump speeds. These are fed in explicitly as bytes because that's what the pump profiles return.

For the light profiles (PWMSlope, PWMParabola, and the one I added recently, PWMSmoothRamp) instead of overloading them I'm leaning towards making them accept arguments in percents as bytes, but output 12-bit integers from 0 to 4096. Then I will know that if a channel is getting a "byte" as an argument it's coming from a pump and it should be interpreted as a percent but if a channel gets an "int" as an argument it's coming from a light profile and the 12-bit int can be scaled down to 8-bit for the Actinic, Daylight, LowATO, or old 6-channel PWM expansion before writing or left alone and just written if it's on the newer 6-channel PWM expansion or 16-channel expansion.

That way everyone gets more levels of dimming for their lights and uses the max capability of whatever chip they have, and they don't have to know that the internals were changed to pass around ints as 12-bit PWM levels rather than percents as bytes. I'd leave alone the various Override functions except to change the GetChannelValue functions to multiply the Override value, which is 0-100, by 4095 and cast it as an int before returning it.

Does that seem reasonable?

[rimai]

You are right, I think the int overload will always be picked up.
But changing the arguments as byte would still not give you the desired result you need.
If you enter arguments in bytes and scale up to int, you loose the resolution you are seeking.
To make it easier for everyone, including you, I think we should just create another function. Something like SetChannelHiRes or SetChannelRaw, which would only work on the new dimming modules.

[AlanM]

I would enter arguments to the lights profiles as byte from 0 to 100 but the instant by instant calculation would be done using 12 bit ints which would get sent to the pwm pins. That's where the resolution matters, not in the input arguments.

[rimai]

Ahh. nice solution

[AlanM]

Are those PWMSlope, PWMParabola functions used as anything other than as input to the PWM write classes that you know of?

If they are exclusively inputs to PWM, maybe no one will notice if the output changes from 0-100 to 0-4096. 8)

[rimai]

I don't think anyone ever used for anything else.

[AlanM]

Thanks for all the help. Off to change stuff and try not to break your cool controller.

[lnevo]

Those functions all take a high and low argument so why should it change anything if you have to specify the arguments? You confused me?

If i say 0,100 i would expect it to still return 0-100...if i said 0,4096 i would not expect a 0-100 return...

[AlanM]

Lee, here's a for instance.

The current version of PWMSlope expect a byte for start, a byte for end. Bytes can only go up to 255. It returns a byte between start and end, just like you said.

However, the function that actually writes the output of the PWMSlope function to hardware is called "Expansion" in the RA_PWM.cpp file. That function makes it obvious that what it actually expects to see coming from the PWMSlope function is not a byte from 0 to 255, but one from 0 to 100. It multiplies the number that you try to set the channel to by 2.55 in the case of the old PWM expansion or by 40.95 in the case of the new one because the new one uses a chip with more bits.

This means that, in the case of the new chip, you're reducing the resolution of that chip by a factor of 41 by using the stock PWMSlope function with bytes in and bytes out from 0 to 100 and getting only 100 levels of dimming rather than 4096 levels. A huge diminishment of capability.

So in order to get that back, I'm going to make the PWMSlope function still accept 0 to 100 as an input for the start and end parameters, but as soon as the function is called with that, I'm going to scale those percentages to 12-bit values between 0 and 4095 and then do all the math with the current time of where I am along the slope. Then I'm going to output a number between 0 and 4095 which can be written directly to the chip with an overloaded version of the Expansion function which accepts ints instead of bytes as arguments without any loss of precision. That version won't multiply the percent by 2.55 or 40.95. It will scale back the 4095 to 255 for the older chipset and write it to the chip.

I'll keep the old byte version of Expansion around to be used by DCPump classes which seem fine to do in percents. I doubt anyone can claim to need 4096 levels of pump speed control, but at low light levels the difference between 1 and 2 out of 100 is really visible while the difference between 1 and 2 out of 4096 is invisible.

[AlanM]

By the way, since I'm keeping it totally separate from the code for the 6 channel expansion, I don't see why someone couldn't have both at the same time.

I thought about trying to make the addresses for the 16 channel one an array so that in the future a whole bunch of them could be added and someone could do something really silly like running 64 channels of LED lights on 4 16ch PWM expansions to make clouds move from side to side or make the sun rise in the east and set in the west. 8)

[Meldrath]

Why not overload the Expansion function to accept an int from an overloaded PWMSlope function? Or...
You could overload PWMSlope and call the expansion function directly from the PWMSlope and not have to deal with conversions per se?

Or will that not work in this case?

Further, I have Kraven's library fully working on my own chips, so if you need any help let me know.

[lnevo]

I agree with overloading Expansion if possible but I haven't looked at the code at all... so I'll leave it to your implementation...

[AlanM]

Overloading Expansion would be fine if all I wanted to do was let people use their own code in their .ino file to write whatever integer they wanted to the 6-channel expansion. It would probably work ok, and I'd have the 16 channel expansion doing 12 bits like I wanted, but that wouldn't help anyone else.

If I overloaded PWMSlope, which I tried to do, to accept either ints or bytes (it currently accepts bytes) the only one that would get called all the time would be the int version unless users explicitly cast the argument to PWMSlope as a byte. When the only version that exists is one that accepts bytes, then the runtime doesn't care if you're giving it an argument of 50, for instance, which is technically an int. It casts it to a byte automatically and does the right thing. However, if now you made two versions, an int version and a byte version, the int one will get called and not the byte. You'd have to do the argument as "(byte)50" in order to get the current byte version, which would violate what I'm trying to do, which is make it work better for everyone with existing .ino files without requiring them to change it all around or learn a new way to do it.

By just overloading Expansion you'd also be stuck with 100 levels of dimming on the relay box and the lowATOport. And you'd just get 100 levels on the convenience functions like PWM.ChannelPWMSlope().

Anyway, TL;DR version, yes it would have been pretty easy to just make a 16 channel version of the Expansion function, make it 12-bit, and call that from overloaded PWMParabola functions, or whatever, it wouldn't help anyone else, so I decided to make the attempt to integrate the 16 channel expansion as fully as possible into the existing codebase. To write in the support for everything that I can figure out that the 6 channel expansion does. And in the process, to change around the internals of how the various PWM chips are called so that everyone's would work a little better.

[Meldrath]

As an fyi, to do that, you're going to have to either create a new call/write methods or you're going to have to change the existing ones. (I went with adding a new expansion/pwm slope/pwm parabola methods)

[AlanM]

Yep. Already done and checked in to my device branch of the reefangel fork on github.

[89delta]

So since the dimming expansion uses the PCA9685 chip that means I could use the Adafruit Servo Breakout Board that I already have correct. Don't care for all 16-channels though as 6 would be plenty. What would I need to do to get it up and running? I already made the usb cable for it with the correct pinouts.

[AlanM]

So since the dimming expansion uses the PCA9685 chip that means I could use the Adafruit Servo Breakout Board that I already have correct. Don't care for all 16-channels though as 6 would be plenty. What would I need to do to get it up and running? I already made the usb cable for it with the correct pinouts.

Seems like you could do that. You'd just have to know what address the Adafruit board is on and tweak the library files appropriately. It should be identical calls to talk to it as it is to talk to the one Roberto makes. It would be the defines in Globals.h for I2CPWM_PCA9685 which by default uses the address of 0x40 for his 6 channel PWM expansion. I don't know which board you have, but I bet it has jumpers to set the address.

That being said, I'm not sure what other customization Roberto does to get the outputs right for the range of things that people want to do with them. For instance, I know he has a way of doing 10V pwm or 5V pwm outputs and also has some filters that can convert those PWM signals into 10V or 5V analog DC voltage signals and he supplies pins to let you just pick which of those you want to have coming out. That's a pretty convenient set of additions.

[89delta]

The default address is 0x40 for the Adafruit Servo Breakout board and 0x41 with A0 jumper soldered. So the board should work without any kind of modifications to it or the library then correct. Thanks for the input/help.

[AlanM]

Quick update on this. The code all compiles and is in my github repo. I've pulled down the upstream stuff from Roberto and merged it all in. I'm going to load it on my RA+ tonight and see what it does over the next couple of days. One thing I discovered is that C++ doesn't like defined names to start with numbers, so I had to replace "16" with "SIXTEEN" everywhere, heh.

Also, Roberto, if you read this, if I changed the RA_Net size from 26 to 42 to accommodate 16 channels of dimming expansion in Globals.h will that break stuff?

[rimai]

If you are going to provide complete compatibility for all 16 channels, there is a lot more to it than just the RANet size.
Anything related to PWM will need to be updated, including a few that pops my mind: PWM channels, PWM override, wifi xml, wifi json, RANet array, LCD screens and internal memory locations.

[AlanM]

OK. Got PWM channels and PWM override already in. I added it in a few small spots of LCD screens variables and internal memory locations, but don't know exactly what I'm doing with those. Didn't do anything to RANet array or the wifi stuff. Will at least look at that.

Thanks.

[AlanM]

What do you think would need to be stuck in the internal memory locations, by the way? I don't see much in there for the regular 6 channel PWM expansion.

Basically I'm finding places where the 6 channel one is referenced and defines for PWMEXPANSION and doing the same thing for the 16 channel one everywhere I can.

[rimai]

Yes, the internal memory has 3 locations for each channel.

Code: Select all

#define Mem_B_PWMSlopeStart0      VarsStart+58
#define Mem_B_PWMSlopeEnd0	      VarsStart+59
#define Mem_B_PWMSlopeDuration0   VarsStart+60
#define Mem_B_PWMSlopeStart1      VarsStart+61
#define Mem_B_PWMSlopeEnd1	      VarsStart+62
#define Mem_B_PWMSlopeDuration1   VarsStart+63
#define Mem_B_PWMSlopeStart2      VarsStart+64
#define Mem_B_PWMSlopeEnd2	      VarsStart+65
#define Mem_B_PWMSlopeDuration2   VarsStart+66
#define Mem_B_PWMSlopeStart3      VarsStart+67
#define Mem_B_PWMSlopeEnd3	      VarsStart+68
#define Mem_B_PWMSlopeDuration3   VarsStart+69
#define Mem_B_PWMSlopeStart4      VarsStart+70
#define Mem_B_PWMSlopeEnd4	      VarsStart+71
#define Mem_B_PWMSlopeDuration4   VarsStart+72
#define Mem_B_PWMSlopeStart5      VarsStart+73
#define Mem_B_PWMSlopeEnd5	      VarsStart+74
#define Mem_B_PWMSlopeDuration5   VarsStart+75

[AlanM]

Ah, right. I didn't make the connection that there were three for each channel.

[AlanM]

These are in and seem to be working well for my custom dimming module. I submitted a pull request against /dev tonight after a few months of work off and on on various things that I considered to be prerequisites to making these work how I wanted them to work. They work in DCPump, PWM, with all of the overrides, and also report over Wifi.

They are currently in https://github.com/amunter/Libraries/tree/issue161 if someone wanted to download the .zip and play, but it should be in the dev branch soon, I imagine.

I recommend that if someone is really interested in seeing what this little controller can do they should take a crack at adding something to the code. I "get it" way more than I would have if I hadn't set out to add a feature and feel pretty confident now that I'll like what I get it to do. 8)

[AlanM]

Oh, and the cool thing is that binder already has it pretty much working in the Android app beta! Talk about service!

[dlplunkett44]

I would really love to have this. Is it completely up and running now? Can all 16 channels be addressed and be dimmed at 12 bit resolution? How can I purchase one and for how much?
THANKS!