-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Add support for the Canis Labs CANPico shield for the Raspberry Pi Pico #97653
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hadn't paid attention to the image file size. I reformatted the original image to more appropriate dimensions and size. Though if you insist, I can replace it with the alternate you proposed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hadn't paid attention to the image file size. I reformatted the original image to more appropriate dimensions and size. Though if you insist, I can replace it with the alternate you proposed.
Given it's basically smaller in size, better quality, and has a transparent background, I might actually insist :) Thanks!
@@ -0,0 +1,171 @@ | |||
.. _mcp251xFD_shield: | |||
|
|||
Microchip MCP2517FD/MCP2518FD CAN bus shields |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unless you're reasonably confident there are/will be other shields using the same chip, it would vote for putting in a folder just named after the shield and have the doc page just focus on that shield
cc @henrikbrixandersen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
This is a dedicated shield for the raspberry pi (unlike e.g. the more generic MCP2515 shields, which reside in a common folder).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kartben @henrikbrixandersen - Hmm ... while I don't plan (read: "need") to implement support for them, the Seed Studio "2-Channel CAN-BUS(FD) Shield for Raspberry Pi" and the MikroElektronika "MCP2518FD Click" with appropriate carrier shield could be added if the current structure is retained.
If you both feel strongly that I should rearrange this for only the CANPico, I will. What is your recommendation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rearrange as already requested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use a dedicated folder and revise the documentation to match.
Add Canis Automotive Labs vendor prefix. Signed-off-by: Steve Boylan <[email protected]>
955ca5d
to
9dc6dbf
Compare
Added some necessary cleanup. |
ac4bcca
to
b5abc2a
Compare
@kartben @henrikbrixandersen - As requested, this has been trimmed to include only the CANPico, and the image has been replaced with the .webp file. |
b5abc2a
to
765b7e0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
transceiver. The CANPico is a carrier board in a ‘sock’ format for a | ||
Raspberry Pi Pico family. | ||
|
||
.. figure:: canis-canpico.jpg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. figure:: canis-canpico.jpg | |
.. figure:: canis-canpico.webp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drat. Fixed.
@@ -0,0 +1,6 @@ | |||
shields: | |||
- name: canis_canpico | |||
full_name: Canis Labs CANPico |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
full_name: Canis Labs CANPico | |
full_name: CANPico |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That change is not consistent with other shields. A selection:
./sparkfun_sara_r4/shield.yml: full_name: Sparkfun Sara R4
./waveshare_epaper/shield.yml: full_name: WAVESHARE e-Paper GDEH029A1
./st_b_cams_omv_mb1683/shield.yml: full_name: ST B-CAMS-OMV MB1683 Camera Shield
./adafruit_vcnl4040/shield.yml: full_name: Adafruit VCNL4040 Proximity and Lux Sensor Shield
./rk043fn02h_ct/shield.yml: full_name: RK043FN02H-CT 4.3" LCD Panel
./mikroe_eth3_click/shield.yml: full_name: ETH3 Click
./lmp90100_evb/shield.yml: full_name: LMP90100 Sensor AFE Evaluation Board
./nrf7002ek/shield.yml: full_name: nRF7002 Evaluation Kit Shield
./npm6001_ek/shield.yml: full_name: nPM6001 EK
./x_nucleo_iks02a1/shield.yml: full_name: X-NUCLEO-IKS02A1 (Standard / Mode 1)
./arduino_modulino_thermo/shield.yml: full_name: Arduino Modulino Thermo
Do you insist on this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ThreeEights
If i need to fight or insist for every request change I think you can agree that this will quickly become exhausting.
Most shields (and boards for that matter) do not have vendor in their name. When they do, it's typically because that's the official / most prominent product name one would find on vendor website, or e.g. in the top results in their favourite search engine.
See e.g. https://www.adafruit.com/product/4161?srsltid=AfmBOooXOsEAM1UFkhs34iq4Nq9wEXZHGZqhDfg3-VhPQAMyL8Xm6Pwm, https://www.sparkfun.com/sparkfun-micromod-asset-tracker-carrier-board.html. Sometiemes it's also to help disambiguate synonyms.
In the case of CANPico, I think you can safely get rid of it but I certainly don't want to spend another 10 minutes justifying my choice or "insisting" for you to agree to apply it :|

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW I also took the liberty to make that comment because I knew you were coming back to this PR anyway. Not sure I understand the rationale of not applying the change while you were there fixing the image file name but oh well :|
Thanks for the PR anyway, this is really cool to see so many new shields added in this release :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kartben - Please have pity on us poor mortals who don't live and breathe Zephyr. I'm not trying to create problems or extra work for anyone here. Try to understand how frustrating this week has been for me: First was the problem with the image. After I corrected it, I was asked to replace it. Alas, I'd not heard of the webp format; asking around our team, two people had heard of it but not used it. Now I've learned that it's the cat's pajamas, the bee's knees, and seems to be the preferred format for Zephyr in the future.
Then came the request to reorganize the files to focus only on the CANPico. I had originally followed the time-honored practice of copying the existing structure for the mcp2815 shields, assuming it would be nice to accommodate other products based on the newer chips. But, of course, pointing that out did not change the opinions of the reviewers.
Now this latest request, which appears to me to be incorrect. I failed to follow your explanation in this case; the two specific examples you mention seem to contradict your explanation:
sparkfun_carrier_asset_tracker/shield.yml: full_name: SparkFun MicroMod Asset Tracker Shield
adafruit_vcnl4040/shield.yml: full_name: Adafruit VCNL4040 Proximity and Lux Sensor Shield
I don't want to waste any more of your time, but please let me know if the current full_name might possibly be acceptable, or can you enlighten me further so my future pull requests will meet the high standards expected for contributions to Zephyr?
And thank you for investing so much effort on this one little piece of this grand edifice!
Steve
765b7e0
to
70099a5
Compare
Add support for shields based on the Microchip MCP251xFD CAN Bus controllers, along with the Canis Automotive Labs CANPico shield for the Raspberry Pi Pico. Updated to focus on only the CANPico shield. Use alternate image for shield. Corrected image file extension. Signed-off-by: Steve Boylan <[email protected]>
70099a5
to
7e1e021
Compare
|
Add support for the Canis Automotive Labs CANPico shield for the Raspberry Pi Pico.