Adding support for 16 channel PWM expansion to libraries

Related to the development libraries, released by Curt Binder
rimai
Posts: 12881
Joined: Fri Mar 18, 2011 6:47 pm

Re: Adding support for 16 channel PWM expansion to libraries

Post by rimai »

lnevo wrote:The override uses a different variable to store the percentage and that variable overrides the default
Correct...
Check this piece of code:

Code: Select all

byte RA_PWMClass::GetChannelValue(byte Channel)
{
	if (ExpansionChannelOverride[Channel]<=100)
		return ExpansionChannelOverride[Channel];
	else
		return ExpansionChannel[Channel];
}
Roberto.
AlanM
Posts: 263
Joined: Wed Jan 01, 2014 7:26 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by AlanM »

rimai wrote:

Code: Select all

#define OVERRIDE_CHANNEL0      2
This define is used by the internal webserver to identify which channel you are overriding when you call http://ipaddress:2000/pox,y
x is the channel and y is the value to override.
So, you will need to add all the 16 channels to the end of the list and make sure to leave OVERRIDE_CHANNELS as the last one.
OK. The way that this one is written, what would happen if, for instance, someone had two of the 6 channel expansions?
AlanM
Posts: 263
Joined: Wed Jan 01, 2014 7:26 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by AlanM »

Also, would I break the non-plus versions for everyone if I changed PWMSlope and PWMParabola and a new one I'd add called PWMSmoothRamp to int's rather than bytes by taking up more space?
rimai
Posts: 12881
Joined: Fri Mar 18, 2011 6:47 pm

Re: Adding support for 16 channel PWM expansion to libraries

Post by rimai »

Only 1 6ch dimming module is supported at this moment.
You should be able to create PWMSmoothRamp. The compiler should be smart enough to not included if the function is never called.
Roberto.
AlanM
Posts: 263
Joined: Wed Jan 01, 2014 7:26 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by AlanM »

OK, thanks.
AlanM
Posts: 263
Joined: Wed Jan 01, 2014 7:26 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by AlanM »

I'm learning about github a bit and talking to my officemate who is an old-time Linux guy about doing commits to github. I had pulled down the dev branch of your repo yesterday to work on it, but didn't fork or anything yet.

What I'm thinking about doing is forking your Libraries dev branch and adding changes a small bite at a time, like just adding a new PWMSmoothRamp function without touching anything else, and then doing a pull request for small parts so it doesn't end up being a big burden on checking things.

Would you be open to pulling in changes like that? I'd probably add that one in, then a few basic functions and defines mentioned above for using the 16ch PWM module, and then some changes to the DCPump (and ReefAngel) to add in the idea of thresholds where the pumps can have values of 0 speed, or threshold to max, but not values inbetween.
binder
Posts: 2871
Joined: Fri Mar 18, 2011 6:20 pm
Location: Illinois
Contact:

Re: Adding support for 16 channel PWM expansion to libraries

Post by binder »

just make sure you do the pull requests on the dev branch. that way it can be tested easier. but sounds like you have the right idea. you will want to fork the repository in order to create the pull requests though.
make small changes and make sure it compiles and runs before you commit and create a pull request. :-)


Sent from my iPad mini
rimai
Posts: 12881
Joined: Fri Mar 18, 2011 6:47 pm

Re: Adding support for 16 channel PWM expansion to libraries

Post by rimai »

The way you think is good :)
Makes it manageable and easy to understand what you are doing.
So, for every bit of things you do, make a new commit and add a comment, that way when I look at the commit history, I know what you did for that particular commit.
You can have a few commits in every pull request too.
Roberto.
AlanM
Posts: 263
Joined: Wed Jan 01, 2014 7:26 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by AlanM »

Great. I think I get it. I'll use the dev branch. I'll fork the Libraries and FeaturesAndUpdateFiles ones because I need to add to feature.txt to get the 16CH stuff working how Roberto suggested above.

I'm a github novice. I've used svn, hg, and cvs at work, but never git or github, so I'm trying to get it right. So far I've only cloned those repos by dragging the URLs to the Windows GitHub client and done my work from there, but I'll fork it and start from there. I tried to hit commit on my cloned version thinking it might be smart enough to fork it at the point where I cloned it and commit the change to my fork, but it just gave me an error because it was apparently trying to commit to your real dev branch.
KRavEN
Posts: 104
Joined: Sun Mar 17, 2013 8:21 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by KRavEN »

I had done code for this in one of my branches a while back if you want to use some of what I did.

https://github.com/brunnels/Libraries/t ... RA_PCA9685

Also changes to Globals and InternalEEPROM

https://github.com/brunnels/Libraries/t ... rnalEEPROM
AlanM
Posts: 263
Joined: Wed Jan 01, 2014 7:26 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by AlanM »

KRavEN, I'll try to figure it out. I was planning to add functions to RA_PWM to mimic the 6 channel ones surrounded by appropriate defines so that the 16 channel module would be familiar to people already running the 6 channel one.

The reason is mostly selfish, on my part, so that I can ask questions about my 16 channel one and get answers from people who know how the 6 channel one is supposed to work. 8) But I think it would also make the code simpler to maintain if it all looks the same.
AlanM
Posts: 263
Joined: Wed Jan 01, 2014 7:26 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by AlanM »

How much would it break if I set the 16 channel one to ints instead of bytes and everyone could be told to cast the outputs of the PWMSlope and PWMParabola and ReefCrest type functions to ints if they want to send them to the 16 channel PWM expansion? Then we could get 12-bit values in there.

I also haven't gotten my head around yet how the 6 channel expansion knows that it's being sent percents when pump modes are called and not raw values out of 255 or how the 6 channel one is using 8 bit numbers at all since I think the chip supports 12bit. Where should I be looking for that stuff?
User avatar
lnevo
Posts: 5430
Joined: Fri Jul 20, 2012 9:42 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by lnevo »

You could always overload the functions :) The only tricky thing is you'll need to cast your arguments as int.
AlanM
Posts: 263
Joined: Wed Jan 01, 2014 7:26 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by AlanM »

OK.

I found the 100-255-4095 mapping part in RA_PWM.cpp

Code: Select all

	
        Wire.write((int)(data*2.55));			// send the data
	Present=Wire.endTransmission();		// stop transmitting
	if (cmd<PWM_EXPANSION_CHANNELS) ExpansionChannel[cmd]=data;
	// Also send data to new module PCA9685
	int newdata=(int)(data*40.95);
rimai
Posts: 12881
Joined: Fri Mar 18, 2011 6:47 pm

Re: Adding support for 16 channel PWM expansion to libraries

Post by rimai »

lnevo wrote:You could always overload the functions :) The only tricky thing is you'll need to cast your arguments as int.
Overloading won't break anything :)
All you have to do is declare the same function with a different size.
For example... We already have this:
SetChannel(byte Channel, byte Value)
You can declare another one like this:
SetChannel(byte Channel, int Value)

This way, when you use the code like this:
byte a=10;
SetChannel (0,a);
It will use the first one.
And if you use like this:
int a=10;
SetChannel (0,a);
It will use the second one.

The compiler is smart enough to find out which variable size you are using and uses the correct one.
Roberto.
rimai
Posts: 12881
Joined: Fri Mar 18, 2011 6:47 pm

Re: Adding support for 16 channel PWM expansion to libraries

Post by rimai »

If you look at the RA_NokiaLCD library, you will see a bunch of overloads of the same function as a reference for you to wrap your head around the idea.
Roberto.
AlanM
Posts: 263
Joined: Wed Jan 01, 2014 7:26 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by AlanM »

Awesome. I did that for the DC pump constructor to add in my idea of a threshold so it doesn't break uses that don't include the threshold argument.
rimai
Posts: 12881
Joined: Fri Mar 18, 2011 6:47 pm

Re: Adding support for 16 channel PWM expansion to libraries

Post by rimai »

I went ahead and merged the request, but had to make a small change due to conflict from a pending merge from brunnels.
It was just a comment he added, so no big deal.
I also changed the default and initial threshold to 30.
Great stuff :)
Roberto.
AlanM
Posts: 263
Joined: Wed Jan 01, 2014 7:26 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by AlanM »

I'm pretty excited to be doing this. 8) Thanks!

I commented on the threshold of 30 thing on github, but I think it should be 0 so that the default is essentially the previous default, which was actually 0. Then current code will work the same as it always did, and if someone wants to set it to 30 to use the new feature, they can. Apparently the vortechs can run all the way down to 0? At least that was lnevo's objection when I proposed adding the threshold right to the wave function outputs.
User avatar
lnevo
Posts: 5430
Joined: Fri Jul 20, 2012 9:42 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by lnevo »

Yeah I use the library modes when setting my vortech in custom modes. The threshold should be set in the dcpump class not in the global function...roberto can you clarify?
AlanM
Posts: 263
Joined: Wed Jan 01, 2014 7:26 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by AlanM »

It is in DC pump not the global. I was just advocating for a default threshold of zero which was the defacto old threshold before there was a new way of calling DC pump.
User avatar
lnevo
Posts: 5430
Joined: Fri Jul 20, 2012 9:42 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by lnevo »

Ah ok. DCPump would be great to change there. The only thing is for people who have chosen very low values due to the threshold can now fix their code or leave it.
rimai
Posts: 12881
Joined: Fri Mar 18, 2011 6:47 pm

Re: Adding support for 16 channel PWM expansion to libraries

Post by rimai »

Yes, the threshold only applies to DCPump, so I think we should leave as 30 to ensure pumps don't go below the minimum and get stuck.
Roberto.
AlanM
Posts: 263
Joined: Wed Jan 01, 2014 7:26 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by AlanM »

Great. I didn't get that the vortech users use a different class.
Meldrath
Posts: 20
Joined: Mon Mar 17, 2014 1:07 pm

Re: Adding support for 16 channel PWM expansion to libraries

Post by Meldrath »

Alan, where are you at on this?
AlanM
Posts: 263
Joined: Wed Jan 01, 2014 7:26 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by AlanM »

Will have the 16 channel functions into RA_PWM by the weekend and then will send a pull request to Roberto. The threshold thing for various DC controlled pumps is in the dev branch now and should work fine, but I have some questions about what the PWM write functions are doing.

Roberto, what is going on in RA_PWM with the code that actually writes the values down the wire? It looks like you first assume that the data is a percentage and you write it to the 8 bit dimmer ports on the relay box (if that's what is at 0x8) and then it gets written to the 12 bit dimmer ports on the 6 channel expansion at 0x40 with the same value? Why are you writing it to two different devices?

Code: Select all

void RA_PWMClass::Expansion(byte cmd, byte data)
{
	Wire.beginTransmission(I2CPWM);  // transmit to device #8, consider having this user defined possibly
	Wire.write('$');				// send the $$$
	Wire.write('$');
	Wire.write('$');
	Wire.write(cmd);				// send the command
	Wire.write((int)(data*2.55));			// send the data
	Present=Wire.endTransmission();		// stop transmitting
	if (cmd<PWM_EXPANSION_CHANNELS) ExpansionChannel[cmd]=data;
	// Also send data to new module PCA9685
	int newdata=(int)(data*40.95);
    Wire.beginTransmission(I2CPWM_PCA9685);
    Wire.write(0x8+(4*cmd));
    Wire.write(newdata&0xff);
    Wire.write(newdata>>8);
    Wire.endTransmission();
}
rimai
Posts: 12881
Joined: Fri Mar 18, 2011 6:47 pm

Re: Adding support for 16 channel PWM expansion to libraries

Post by rimai »

0x8 is the old dimming expansion module that was 8bit resolution. 0x40 is the new dimming expansion module. We have to write to both because we don't know which module someone would have.
Roberto.
AlanM
Posts: 263
Joined: Wed Jan 01, 2014 7:26 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by AlanM »

Ah, got it, thanks.
AlanM
Posts: 263
Joined: Wed Jan 01, 2014 7:26 am

Re: Adding support for 16 channel PWM expansion to libraries

Post by AlanM »

Roberto,

Is there a way to detect which 6 port expansion the user has with read or something to see if there's something at address 0x8 or 0x40?

I'm trying to overload some of the writing functions for the regular 6-channel PWM expansion with integers as well as bytes to allow for the smoother dimming that would come from the 256 or 4096 PWM levels, but I'm having trouble with some of the functions and not knowing whether I should multiply the input by 2.55 or by 40.95 when setting a percent dimming.
rimai
Posts: 12881
Joined: Fri Mar 18, 2011 6:47 pm

Re: Adding support for 16 channel PWM expansion to libraries

Post by rimai »

The easiest way for you is to create two overloads.
One for 8bit and another for 12bit.
The 8bit would be like this:
SetSmoothChannel(byte channel, byte value)
And the 12bit would be like this:
SetSmoothChannel(byte channel, int value)
The compiler is smart enough to distinguish what you are using.
Roberto.
Post Reply