PizzaProgram / MCP23017-PCF8574-AIO

This is a Node-Red node to communicate with MCP23017 or PCF8574(A) chips on a I2C bus. Both Input + Output.
https://www.npmjs.com/package/@pizzaprogram/mcp-pcf-aio
MIT License
2 stars 1 forks source link

Code question js-file #8

Closed Joe-ab1do closed 1 year ago

Joe-ab1do commented 1 year ago

As you may remember, I have been upgrading your excellent code to expand the types of port expanders to include mcp23008 and pcf8575. Most of the work has been focused on the html-file, but I am now going through the js-file and have a question: in line 293 you have: aBus.writeByteSync(_parCh.addr, BNK0_IOCON_B , 0xA0) followed in line 297 by aBus.writeByteSync(_parCh.addr, BNK1_IOCON_A , 0xE4)

So I am confused: why write to BNK0_IOCON_B (IOCON.BANK=0) and then BNK1_IOCON_A, (IOCON.BANK=1) especially as in line 75 you write that only IOCON.BANK=1 will be used?

All I can think of is the statement in line 293 is just in case the chip happens to be in "register sequential" mode, you set it to "bank sequential" mode by writing 0X0A (BANK = 1) to address 0X0B (=BNK0_IOCON_B). If the chip is already in "bank sequential mode", address 0X0B does not exist and line 293 will have no effect. Now that the chip is in "bank sequential" mode, line 297 keeps BANK=1, activates MIRRORed interrupts and sets interrupt pin to an Open Drain Output. Is this correct?

Thanks, Joe

PizzaProgram commented 1 year ago

in case the chip happens to be in "register sequential" mode, you set it to "bank sequential" mode

Yes, that was the only way to make sure the chip is in the right mode, to switch it 2x. I've made it during initialisation only once, but can not remember the line numbers from head. ;-)

PizzaProgram commented 1 year ago

I've also made a note at this line...

OFF: It seems that github handles TABs differently than VSCode. My code looks like a mess here.

I'm very glad that there is at least 1 person on this planet who reads my actual code :-) Thanks for enhancing it! I hope I'll have more time in a month, so I can look into your "html upgrades", and maybe add it here too. What is the link to your repository?

Originally I didn't wanted to set different (8 / 16) maximums at html level because:

Joe-ab1do commented 1 year ago

I've also made a note at this line...

No worries about how the code looks - I too am looking at it in vscode. I did see that note and the lines below, but it was not clear to me why first write to a register address that only exists if the chip is in 'register sequential mode', as right at the top of the js-file the comments state the program does not use that mode. Glad we cleared that up :-). I am expanding the comments around lines 293 and below, to better clarify what exactly is happening.

I'm very glad that there is at least 1 person on this planet who reads my actual code :-) Thanks for enhancing it! I hope I'll have more time in a month, so I can look into your "html upgrades", and maybe add it here too.

Get ready for lots more comments/questions as I will be going through your code line by line, so that I exactly understand what is going on. I need to do this to be able to add the other chips, so bear with me.

I guess I know a little more about html coding, but by no means an expert.

maybe there are chips that work the same way as these, but can handle less/more pins? (And it seems, I was righ! See this: https://github.com/PizzaProgram/MCP23017-PCF8574-AIO/issues/2)

Now that's funny, as that is a link to a question I asked! Nice to be referenced to oneself :-)

The link to my repository where I am doing work on this is a fork off your repository and can be found here . I have not updated it in a while, because I am working on being able to select the i2c-bus from available buses, which looks like it is working. This still needs a little work.

Also, I think I have come up with a way to stop pins on a chip being selected twice, by blocking it from selection. I already do this for MCP23017 input pins 7 and 15, as they can only be used as outputs according to the spec sheet.

If this progresses to where I want it to go, maybe we should change the name to MCP230XX-PCF857X-AIO. Just a thought...

PizzaProgram commented 1 year ago

OFF:

Now that's funny, as that is a link to a question I asked! Nice to be referenced to oneself :-)

:-D :-D :-D ... sorry, had only 5 hours of sleep.

I am working on being able to select the i2c-bus from available buses Cool!

repository can be found here

Thanks, will look into it, after you updated. Please notify me!

MCP230XX-PCF857X-AIO

Sounds good. I have no problem with it. Although yet do not know how much plus work it is causing. The last change took me a while... Search engines of Node-RED repo will find directly anyway.

Joe-ab1do commented 1 year ago

In the js-file it looks like you create a boolean global context variable (i2c#RW, where # stands for the i2c-bus number) to keep track of whether a read/write operation is happening (True) or not (False). Is this correct? (I have not been able to use your node yet because I only have mcp23008 chips, so cannot verify the creation of the global context variable).

image

Also it looks like you create an array holding the (node-red generated) ids for all the caller nodes (input or output) related to a chip and that as an input/output node is created and linked to a chip, its id is added to the ids array.

image

I am assuming the ids array index equals the bitNum, e.g. if a node is created for bitNum = 3, that node's id is entered in ids[3]?

PizzaProgram commented 1 year ago

1.

Contexts are used, to check for wrong user setup, so nobody can accidentally set the SAME bus number and address for 2 components.
You don't need to have more chips physically to test it, just create a 2th chip and try to set the same bus + addr numbers.
... than see at the logs what is happening. It will skip the 2th one, not initialising it.

2.

... and linked to a chip, its id is added to the ids array.

The IDs are the PINs of 1 chip. Not for other chips.

3.

I am assuming the ids array index equals the bitNum, e.g. if a node is created for bitNum = 3, that node's id is entered in ids[3]?

YES! :-)

4.

It seems your VSCode tabulates things bad too. Maybe you have downloaded the code from github? All = signs on that picture should be lined under each other nicely. :-(

PizzaProgram commented 1 year ago

important:

If you want to be able to handle other chips too, the whole concept of line 174 (maxBits) must be reconstructed. Currently the code is using a very simple way to determine what kind of chip is it using. If there are more than just 2 types, you have to be able to add special cases and handle those too.

Joe-ab1do commented 1 year ago

It seems your VSCode tabulates things bad too. Maybe you have downloaded the code from github? All = signs on that picture should be lined under each other nicely. :-(

The image is actually from your code from gitHub, not vscode. Sorry about that, but my code has already changed so much the line numbers no longer match those of your code.

If you want to be able to handle other chips too, the whole concept of line 174 (maxBits) must be reconstructed. Currently the code is using a very simple way to determine what kind of chip is it using. If there are more than just 2 types, you have to be able to add special cases and handle those too.

Way ahead of you: I have already modified the code so that the chipType (this.chipType has been added as a new variable) is selected separately from the i2c-address. So from a drowdown the user selects the chipType (mcp23008, mcp23016, pcf8574, pcf8574A or pcf8575). From a separate dropdown the user selects the i2c address (0x20-27 for all chipTypes except for PCF8574A: 0x38-0X3F). The range of selectable addresses changes depending on the selected chipType. This all works :-) So I no longer rely on the doubling of address to determine what kind of chip has been used.

So in my case I can now use: this.MaxBits =(this.chipType==="MCP23017" || this.chipType==="PCF8575") ? 16 : 8; :-)

PizzaProgram commented 1 year ago

Perfect !

PizzaProgram commented 1 year ago

... but you still need to use a context to determine if no chips are configured to the same address. Except if you can block that somehow during design/config time?

Joe-ab1do commented 1 year ago

Yeah, I know. I haven't changed any of the runtime code in that respect, so that should all still all work (I hope!) I have an idea on blocking selection during design/config, just haven't had time to work it out yet...

Joe-ab1do commented 1 year ago

OK, I've made a lot of progress

I have gone through all the runtime code and modified it such that instead of checking whether addr>0X40 or not, the chipType is used. This ended up being pretty easy for the MCP23008 as it is a much simplified version of the MCP23017. The PCF8575 code I'm a little unsure about: it works much like the PCF8574 (no registers to set), but instead of sending/receiving 1 byte when writing/reading to it, it sends/receives 2 bytes (LSB then MSB). Not quite sure what i2c-bus statement to use for that. For now I've settled on e.g. _aBus.i2cWriteSync(_addr,2,ip16 & 0xFFFF); and _aBus.i2cReadSync(_addr,2,ip16). Will need to see if that works. Also don't know yet if, when receiving LSB followed by MSB, any byte manipulation will be required.

I will be looking at bus number/address combinations too so that no two chips can be on the same bus/address. If that works, then no need for runtime to keep track of context.

Joe-ab1do commented 1 year ago

Another comment: wondering if a separate invert option is required: isn't it just easier to add a change node that inverts the incoming/outgoing message. That way invert becomes explicit and is visible in the editor. Just wondering what your thoughts are on this?

PizzaProgram commented 1 year ago

The design/config also blocks pin numbers 7 and 15 (MCP23008/MCP23017) from being selected as inputs. They can be selected as outputs

WHY did you do that??? Please undo it.

It is rare that someone is using a chip mixed. I'm using 16 pins on one chip only for relays, and an other 3x16 for buttons.

PizzaProgram commented 1 year ago

so it seems to me that the runtime code can be dramatically simplified.

With my code you can set pins dynamically at runtime too. No need to pre-configure the pins at design time. Enough to put only 1 node to the pane. So it still needs to keep track of everything during runtime.

PizzaProgram commented 1 year ago

isn't it just easier to add a change node that inverts the incoming/outgoing message.

No, it is not! Some relay boards are at "floating state" when powered on, and the first time you have to set opposite direction of a pin to keep them OFF. (5V is connected and needs ground = 0V to "turn ON".)

That's why it is very important, that it will be initialised the inverse way from the beginning.

I've started the whole development of this chip only because of that! (Arduino + Firmata could not do it, whenever I've powered on my RPi, all my roller shutters started to move and all engines started to run, also all light turned on. This can be very dangerous, basically it could kill a kid, if it plays under the metallic shutter and the power comes back after an outage.)

Joe-ab1do commented 1 year ago

WHY did you do that??? Please undo it.

See the note on page 18 of the MCP23017 spec sheet regarding register IODIR Note: For MCP23017, IO7 must be set via I 2 C interface to "0" (output)

and on page 9 of MCP23008 spec sheet: Note: For MCP23008, IO7 must be set via I 2 C interface to "0" (output)

For MCP23017 the note refers to both IODIRA and IODIRB, so it clearly states that pins 7 and 15 can be output only. Either that, or the spec sheet is wrong. (MCP23008 only has one IODIR register).

Have you tried using pin 7/15 as an input?

Enough to put only 1 node to the pane.

Didn't realize that - will need to look into that further. Having said that, my personal preference is to use a single node for each input/output because it makes everything visually explicit, rather than burying commands in messages, such as choosing which output/input to use, together with probably lots of function nodes where the appropriate message is created. I will admit, it does fill up the editor pane with lots of nodes and wires, but using link nodes can usually take care of the wire spaghetti that might otherwise occur. Maybe an alternative would be to add something like "message driven" as an option to the pin selection? That way only one node could be used for a whole chip (or 2 if the chip has both inputs and outputs) and everything is determined by the message it receives/sends. Selecting "message driven" would then not allow other (input/output) nodes for the same chip to be on the editor pane. So the user can then choose either a single "message driven" input/output node, or one node for each pin of a chip, but not both.

Some relay boards are at "floating state" when powered on, and the first time you have to set opposite direction of a pin to keep them OFF. (5V is connected and needs ground = 0V to "turn ON".)

I actually have one of those boards, which I drive with the RPi's GPIO ports. Rather than invert everything, the GPIO output node has an option to initialize the output. If selected, then the port can initialize either high or low. Seems to me we could do that here too. Because I originally designed my flow expecting the relays on the board to use positive logic (on = high), simply adding an invert node right before the GPIO node changed it to negative logic (on = low) and because the GPIO port initializes to high, relays are all off when everything is turned on. Also no need to mess with invert on input ports.

Joe-ab1do commented 1 year ago

Enough to put only 1 node to the pane.

I thought about this a little more and it seems to me this only makes sense if all the ports on a chip are either inputs or outputs, which is the situation you described you have. Then for the chips with only outputs you would put just 1 output node on the editor pane, and for those with only inputs just 1 input node. Is this indeed how it works? If that is indeed the case then an check box labeled something like "all outputs" (or "all inputs" as the case would be) would hide the port selection drop down list and port selection would be determined dynamically by message.

Also, can/do you indeed use pins 7 and 15 (A07 and B07) as inputs on MCP23017?

PizzaProgram commented 1 year ago

Is this indeed how it works?

Yes, my code works currently this way too!

can/do you indeed use pins 7 and 15 (A07 and B07) as inputs on MCP23017?

I think so, but have to confirm that. As I've already explained: I have no monitor for my RPi now, and not much free time to work on this project. Maybe in January.

PizzaProgram commented 1 year ago

If selected, then the port can initialize either high or low. Seems to me we could do that here too.

No need. My current code works fine. Worked on it 3 month long to achieve that. Do not trust the chip, trust NodeRed. (I talk from experience!)

OFF: Sometimes, when I read your posts, I have the feeling like You act similar than the new "Twitter owner" ... First you destroy everything, to realise later: it was good how it was. :-D

PizzaProgram commented 1 year ago

See the note on page 18 of the MCP23017 spec sheet regarding register IODIR Note: For MCP23017, IO7 must be set via I 2 C interface to "0" (output)

I've checked my PDF, and there is nothing like that there. kép

PizzaProgram commented 1 year ago

At the end of my MCP23017 PDF is: 06/23/16 DS20001952C-page 42 2005-2016 Microchip Technology Inc.

Links are provided on the main page.

PizzaProgram commented 1 year ago

Also there are ca. 100 pages about this chip and nobody ever mentioned any IO7 restriction.
Actually there is a "chessboard project" too, using 4x16 = 64 inputs.

Joe-ab1do commented 1 year ago

Hmmmm, seems like MicroChip changed something?

Here's a copy of my pdf with the Note highlighted:

MCP23017 spec sheet

You'll also note the version DS20001952D and 2005-2022 Microchip Technology Inc at the bottom of the page. MicroChip updated the spec sheet in 2022 (see below). Key question is, of course, did they change the chip or was there an error in the earlier spec sheet. If the former, there are 2 versions of the chip out there, which would then be very confusing!

The revision history on page 37 shows the following:

Revision D (June 2022) • Updated description in Section “Features”. • Updated Table 2-1. • Updated drawings for MCP23017 in Section “Package Types”. • Updated Section 3.5.1 “I/O Direction Register”. • Updated Section 4.1 “Package Marking Information”. • Updated Section “Product Identification System”, with Automotive Qualified devices. • Minor text and format changes throughout

I guess only testing of the actual device will tell.

Joe-ab1do commented 1 year ago

Not to beleaguer the point, but this is what my page 1 shows:

MCP23017 spec sheet page 1

Note the orange rectangles which highlight in the Features and the pin-out image that pins A7 and B7 are output only. I'm getting the sneaking suspicion that there may be a silicon revision per June 2022! I just can't find confirmation of that.

Joe-ab1do commented 1 year ago

I have contacted microchip regarding the modification and will see what they come back with.

OFF: Sometimes, when I read your posts, I have the feeling like You act similar than the new "Twitter owner" ... First you destroy everything, to realise later: it was good how it was. :-D

Cute, but at this point I have not "destroyed" anything. Your code may well work fine for you and others that just use the mcp23017/pcf8574(A) chips, where i2c address doubling is used as an implicit mechanism to distinguish between the two . In fact the code, originally written by @willhey, which you yourself improved upon, I am just trying to further improve by expanding the port expander family to include mcp23008 and pcf8575. I appreciate you may not use these chips, but some of us do. My philosophy is also to catch/avoid errors as soon as possible, preferably at the design/config stage before deploying. In the process, I have discovered that Node-Red does a lot of work behind the scenes to enable this, so we might as well make use of it.

I am just trying to keep you in the loop during this process, but if you don't want to be, just let me know.

PizzaProgram commented 1 year ago

OFF:

... I am just trying to keep you in the loop during this process, but if you don't want to be, just let me know.

Sorry, I think you have misunderstood my "funny me". Probably my bad english. I'm very happy you improve this code!
I WILL use this chip too, I just had no time to connect to my house. Yet. (It's all on my desk since 1+ year instead of hooked up to my buttons and roller shutters.)

Joe-ab1do commented 1 year ago

No problem and happy you want to stay in the loop :-)

I WILL use this chip too, I just had no time to connect to my house. Yet.

Yeah, I know how that goes. I still have several projects lying around here too Not so much to automate my house, as to control my amateur radio station (antenna switching, responding to switch inputs/relay closures, antenna rotators, tuning devices, A/D conversions, amplifier and receiver/transmitter control, etc, etc, etc).

No response from microchip yet, but then we are approaching the Holidays...

PizzaProgram commented 1 year ago

Tip: There is an other repo made for Arduino: link It is always up to date, many programmers work on it. If you understand C++ , you can learn a lot from it.

Maybe we could ASK them about the "Pin 7+15 Output only" thing... if you have a few minutes to start a conversation there.