crycode-de / node-pcf8574

Node.js module for controlling each pin of a PCF8574/PCF8574A I2C port expander IC.
GNU General Public License v2.0
1 stars 2 forks source link

#55 Issue fix - Updates to support publish of new version. #56

Closed lynniemagoo closed 1 year ago

lynniemagoo commented 1 year ago

As the counters are specific to pcf857x instances, I would recommend we create a sibling static object store and keep the counters local to the class and not decorate Gpio with our fields.  However, consider that I’m writing a CAT9555 class and want to share same gpio between 2 pieces of hardware.  That is not possible today and is only possible if the cat9555 code reuses a common counter field on a gpio.  If we were to rename the counter field to a name known by both expander types.  This is a larger conversation that I’d like to have via email with you.  For now, if you are good, can we push the new version?Thanks,LyndelSent from my iPhoneOn Apr 12, 2023, at 2:09 AM, Peter Müller @.***> wrote: @crycode-de approved this pull request.

Looks good to me know. Thank you! I think it's fine to store the gpio pin in our own instance. Adding properties to foreign objects isn't a good practice in general. Since we know remember the gpio pin number ... maybe we should better store the use counters in a static object (like the used gpios)?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

lynniemagoo commented 1 year ago

I just reread your reply about where to store the counters.  Add your suggested declaration of the counter object store as a comment on the PR and I will make the change by Sunday. Thanks,lyndelSent from my iPhoneOn Apr 12, 2023, at 7:32 AM, Lyndel McGee @.> wrote:As the counters are specific to pcf857x instances, I would recommend we create a sibling static object store and keep the counters local to the class and not decorate Gpio with our fields.  However, consider that I’m writing a CAT9555 class and want to share same gpio between 2 pieces of hardware.  That is not possible today and is only possible if the cat9555 code reuses a common counter field on a gpio.  If we were to rename the counter field to a name known by both expander types.  This is a larger conversation that I’d like to have via email with you.  For now, if you are good, can we push the new version?Thanks,LyndelSent from my iPhoneOn Apr 12, 2023, at 2:09 AM, Peter Müller @.> wrote: @crycode-de approved this pull request.

Looks good to me know. Thank you! I think it's fine to store the gpio pin in our own instance. Adding properties to foreign objects isn't a good practice in general. Since we know remember the gpio pin number ... maybe we should better store the use counters in a static object (like the used gpios)?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

crycode-de commented 1 year ago

Yeah the handling of the counters will be an other topic. I'll merge this PR know and create the v3.1.0 release today.