Skip to content

Conversation

@CD2468
Copy link
Contributor

@CD2468 CD2468 commented Jul 26, 2021

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Proposed changes

It was only possible to communicate with one MCP3008 at once.
Now several MCPs can be used at the same time.

  • Add the possibility to select CS 2 in the options.
  • The node displays now bus, device and cannel number
  • Modified error message

Checklist

  • I have read the contribution guidelines
  • For non-bugfix PRs, I have discussed this change on the forum/slack team.
  • Link to Forum
  • I have run grunt to verify the unit tests pass
  • I have added suitable unit tests to cover the new/changed functionality

Copy link
Member

@dceejay dceejay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of tiny questions to fix but yes basically good...

module.exports = function(RED) {
"use strict";
var fs = require('fs');
var fs2 = require('fs');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why we need this duplication ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a holdover from debugging.
The line can remain original

// unlikely if not on a Pi
try {
var cpuinfo = fs.readFileSync("/proc/cpuinfo").toString();
var cpuinfo = fs2.readFileSync("/proc/cpuinfo").toString();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise - why won't the original be ok ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line can remain also original.

this.dnum = parseInt(n.dnum || 0);
this.bus = parseInt(n.bus || 0);
this.dev = n.dev || "3008";
this.fs = require('fs');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent ->

Copy link
Contributor Author

@CD2468 CD2468 Jul 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I was not shure if the definition from the global place caused any problems. So I createt a new one and renamed the orginal to fs2.
If you want to remove the line you have also edit line 42.

from:
this.fs.statSync("/dev/spidev"+node.bus+"."+node.dnum);
to:
fs.statSync("/dev/spidev"+node.bus+"."+node.dnum);
I testet this on my raspberry and it worked

CD2468 added 2 commits July 30, 2021 10:01
implementation of the discussion
@CD2468 CD2468 requested a review from dceejay August 6, 2021 15:39
@linux-foundation-easycla
Copy link

CLA Not Signed

@dceejay
Copy link
Member

dceejay commented Oct 27, 2021

/easycla

@dceejay dceejay merged commit ae4b6bb into node-red:master Oct 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants