Skip to content
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

Rename AP_Logger_DataFlash to AP_Logger_JEDEC #26322

Merged

Conversation

peterbarker
Copy link
Contributor

The naming if this driver class is really confusing!

This is just one of the dataflash backends we support, the other being the W25N01GV.

Done as two commits, one to rename the files, the second to adjust for the renaming.

Board                    AP_Periph  blimp  bootloader  copter  heli  iofirmware  plane  rover  sub
CubeOrange-periph-heavy  *                                                                     
Durandal                            *      *           *       *                 *      *      *
Hitec-Airspeed           *                                                                     
KakuteH7-bdshot                     *      *           *       *                 *      *      *
MatekF405                           *      *           *       *                 *      *      *
Pixhawk1-1M-bdshot                  *                  *       *                 *      *      *
f103-QiotekPeriph        *                                                                     
f303-Universal           *                                                                     
iomcu                                                                *                         
revo-mini                           *      *           *       *                 *      *      *
skyviper-v2450                                         *                                       

Copy link
Collaborator

@andyp1per andyp1per left a comment

Choose a reason for hiding this comment

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

I don't agree with this naming change. JEDEC is an organization that controls many, many specs. Why is DataFlash confusing? I think you should pick a name that actually makes sense. I would, for instance, be ok with AP_Logger_Flash.

@peterbarker
Copy link
Contributor Author

I don't agree with this naming change. JEDEC is an organization that controls many, many specs. Why is DataFlash confusing? I think you should pick a name that actually makes sense. I would, for instance, be ok with AP_Logger_Flash.

Is W25N01GV not a flash chip?!

Would you be happy with AP_Logger_Flash_JEDEC instead?

@peterbarker
Copy link
Contributor Author

@magicrub
Copy link
Contributor

How about black box?

@peterbarker
Copy link
Contributor Author

How about black box?

Rather too generic, I think? This is a driver for hardware which speaks a specific set of opcodes, you could foresee other standards in use there.

@peterbarker
Copy link
Contributor Author

So I spent a couple of hours trying to hunt down the standard these opcodes follow.

Couldn't find one.

I've seen them references as "Industry Standard Opcodes", frequently by the JEDEC name ("JEDEC READ ID"), and sometimes by one of the original implementers ("M25P80 opcodes").

I was fully expecting to be able to reference a JEDEC standard number, but I can't find one.

Linux has gone with "spinor" to describe them: https://github.com/torvalds/linux/blob/master/include/linux/mtd/spi-nor.h#L21

So we could make this AP_Logger_Flash_SPINOR....

@peterbarker peterbarker force-pushed the pr/logger-rename-jedec-to-dataflash branch from da7198c to 2a172f3 Compare February 28, 2024 07:40
@peterbarker peterbarker merged commit 6f4ab02 into ArduPilot:master Feb 28, 2024
92 checks passed
@peterbarker peterbarker deleted the pr/logger-rename-jedec-to-dataflash branch February 28, 2024 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants