Pull Requests - bus pirate 5 firmware

New Pull Request: PR to fix some problems with the storage
phdussud/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

Bunch of fixes to fix the hand over of the file system changes between BP and the host (At least Windows).
Support for Windows device eject command.
After all file system object writes, the BP mass storage is ejected and re-inserted to force Windows to notice the changed storage.
By analyzing the USB MSC traffic between a well known Trenscend SD card reader I found out and implemented the correct sequence of SENSE CODEs to emit during ejection and insertion of a changed storage system.

Created by: phdussud

1 Like

New Pull Request: Update build.yaml
DangerousPrototypes/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

Do not fail fast … try to complete all builds even if one fails.

Temporarily remove windows-latest due to continued build failures.

Created by: henrygab

New Pull Request: Langauge selection options in language they represent
henrygab/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

  • Translation selection should be in language it represents

json2h.py

  • Prevent accidental translation of language selection text using regex (to avoid constant need to update this script).
  • Explicitly require Python 3.7 or higher
  • Warn about non-compliant identifiers
  • Tabs to spaces in generated .h files
  • Use functions to logically separate code portions
  • Provide progress output
  • output fixed-width key (also helps highlight overlong identifiers)

*.json

  • Remove unused key/value pairs. Generated .h files were unchanged, confirming non-use

Created by: henrygab

New Pull Request: Resolve PR issues and fix a deadlock issue introduced by the diskio_mutex
phdussud/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

Sorry if I am late. I addressed the PR issues and found a problem with the mutex i introduced.

Created by: phdussud

New Pull Request: sfud: Add M25P128
cpackham/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

Add the 16MB SPI-NOR M25P128. This chip doesn’t support SFDP so it needs to be looked up based on the RDID information.

The memory is organized as:

  • 16777216 bytes (8 bits each)
  • 64 sectors (2 Mbits, 262144 bytes each)
  • 65536 pages (256 bytes each).

Created by: cpackham

New Pull Request: Use accessor function for translated strings
henrygab/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

Resolves #112 - Use Accessor functions for translated strings

Created by: henrygab

New Pull Request: Added handling of host lock and release
phdussud/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

Based on a suggestion from HenryGab, this PR honors the SCSI lock / release

Created by: phdussud

New Pull Request: Safe strings phase 1
mbrugman67/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

This is the first phase of removing unsafe string functions as per Issue #98.

This change marks the sprintf() function as deprecated; as such, each usage will result in a compiler warning during build. This is only a warning, there will be no change to existing functionality in compiled objects or linked executables/artifacts. You may safely ignore these warnings in your build output.

Example warning:

[ 52%] Building C object CMakeFiles/bus_pirate5_rev10.dir/binmode/sump.c.obj
./BusPirate5-firmware/binmode/sump.c: In function 'sump_do_meta':
./BusPirate5-firmware/binmode/sump.c:146:5: warning: 'sprintf_' is deprecated [-Wdeprecated-declarations]
  146 |     sprintf(cpu, "RP2040 %uMhz", sysclk / ONE_MHZ);
      |     ^~~~~~~
In file included from /home/matty/data/BusPirate5-firmware/pirate.h:78,
                 from ./BusPirate5-firmware/binmode/sump.c:39:
./BusPirate5-firmware/printf-4.0.0/printf.h:90:17: note: declared here
   90 | #define sprintf sprintf_

(I hope this doesn’t make me the least popular contributor to the project :slightly_smiling_face: )

Over the next couple of weeks I will update the codebase to replace sprintf() with the safter snprintf() until all warnings are gone. After that, using sprintf() will result in a compile error instead of a warning to prevent accidental reintroduction.

Created by: mbrugman67

2 Likes

New Pull Request: Add code to break the host_lock after a second
phdussud/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

In response to the problem on Linux, I propose to break the host_lock after a second timeout.

Created by: phdussud

2 Likes

New Pull Request: Safe strings phase 2
mbrugman67/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

It was my intention to break this into a few smaller PRs, but the nature of the code and method dependencies in ui_statusbar.c didn’t really make that feasible.

So here’s the code with all sprintf() calls changed to snprintf(). There is also the marking of sprintf() with an attribute that will generate a compile error if it is used again. For example:

[  7%] Building C object CMakeFiles/bus_pirate5_rev10.dir/ui/ui_statusbar.c.obj
In file included from ./BusPirate5-firmware/pirate.h:78,
                 from ./BusPirate5-firmware/ui/ui_statusbar.c:4:
./BusPirate5-firmware/ui/ui_statusbar.c: In function 'ui_statusbar_info':
./BusPirate5-firmware/printf-4.0.0/printf.h:90:17: error: call to 'sprintf_' declared with attribute error: Use of sprintf() not allowed; please use snprintf() instead
   90 | #define sprintf sprintf_
./BusPirate5-firmware/ui/ui_statusbar.c:30:9: note: in expansion of macro 'sprintf'
   30 |         sprintf(buf, "stuff");
      |         ^~~~~~~
gmake[3]: *** [CMakeFiles/bus_pirate5_rev10.dir/build.make:1406: CMakeFiles/bus_pirate5_rev10.dir/ui/ui_statusbar.c.obj] Error 1
gmake[2]: *** [CMakeFiles/Makefile2:1857: CMakeFiles/bus_pirate5_rev10.dir/all] Error 2
gmake[1]: *** [CMakeFiles/Makefile2:1864: CMakeFiles/bus_pirate5_rev10.dir/rule] Error 2
gmake: *** [Makefile:150: bus_pirate5_rev10] Error 2

The developer should see the line error: call to 'sprintf_' declared with attribute error: Use of sprintf() not allowed; please use snprintf() instead and realize they need to use the safer method.

Created by: mbrugman67

3 Likes

New Pull Request: DO NOT MERGE - Storage architecture working branch
henrygab/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

UNTESTED - For early review and comments only.

Created by: henrygab

New Pull Request: quiet a long-standing compiler warning
henrygab/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

The following is the compiler warning:

./commands/spi/sniff.c: In function 'sniff_handler':
./commands/spi/sniff.c:78:8: warning: 'ui_help_show' reading 4 bytes from a region of size 0 [-Wstringop-overread]
   78 |     if(ui_help_show(res->help_flag,usage,count_of(usage), &options[0],count_of(options) )) return;
      |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./commands/spi/sniff.c:78:8: note: referencing argument 2 of type 'const char * const[0]'

In file included from ./commands/spi/sniff.c:17:
./ui/ui_help.h:10:6: note: in a call to function 'ui_help_show'
   10 | bool ui_help_show(bool help_flag, const char * const usage[], uint32_t count_of_usage, const struct ui_help_options *options, uint32_t count_of_options);
      |      ^~~~~~~~~~~~

I’ve scratched my head trying to understand where the invalid read is coming from, but in the end, just ensuring there’s some help string here is a safe enough fix.

Created by: henrygab

New Pull Request: Move sources to subdirectory ./src
henrygab/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

Also:

  • CMakeFile.txt has single file per line
    • This helps visually find referenced files
    • It also helps grep / search and replace / etc.
  • Updated .github/build.yaml to include additional files into artifacts, for debugging

Created by: henrygab

New Pull Request: Cleanup
henrygab/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

The only code impact is the addition of some assertions.

Those insertions are in place to help early-detect potential deadlock situations, such as if core1 calls printf() or outputs to binmode.
These situations would cause deadlock if the buffer were full (e.g., from Core0’s earlier output).

Created by: henrygab

New Pull Request: Add summary to bug e9
henrygab/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

Fixes #105

Created by: henrygab

return bug_seems_fixed; :rofl:

2 Likes

New Pull Request: Devcontainer users
wyattearp/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

This attempts to address concerns from @henrygab about the container running in privileged mode as well as provide access to USB devices (at least in Linux) correctly the the .devcontainers system as part of VScode.

It also introduces the concept of the .env file used for debugging, this way if anyone has multiple BPs or some weird udev rules, they’ll be able to specify which device they’d like to talk to.

The default user in the container is now also no longer root but moved to a build user with no password, but sudo permissions. This will allow you to add new things to your container if you want (say you were missing some strange python package). This can be customized if you want to build your own container, but uses the assumption that most people probably are running with their default userid/groupid.

Created by: wyattearp

1 Like

New Pull Request: Powershell script to which serial ports are for which function
henrygab/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

Usage

  1. Open a Windows Powershell command prompt
  2. Change to the directory containing the powershell script
  3. Review the contents of the script (for malicious content)
  4. If necessary, permit the loading of the # execute as follows:
  5. If satisfied with content, permit the loading of the script:
    Set-ExecutionPolicy -ExecutionPolicy RemoteSigned -Scope Process
  6. Load the script into your current Powershell process
    . .\show_port_info.ps1
  7. Run the main function that the script defined
    Get-BusPirateDetails

Example output

The following is an example, with a single BusPirate5 device connected:

PS C:\Users\ian> Get-BusPirateDetails
InstanceId          : USB\VID_1209&PID_7331\FEDCBA9876543210
Serial              : FEDCBA9876543210
Location            : Port_#0003.Hub_#0007
InstanceID_Terminal : USB\VID_1209&PID_7331&MI_00\8&32E00187&0&0000
Port_Terminal       : COM55
InstanceID_Binary   : USB\VID_1209&PID_7331&MI_02\8&32E00187&0&0002
Port_Binary         : COM56
InstanceId_Storage  : USB\VID_1209&PID_7331&MI_04\8&32E00187&0&0004

In the above example output, the terminal is available on COM55, while
the binary serial port is available on COM56. The serial number of each
device is also shown.

Problems

If not getting the expected results, execute the script with more verbose output,
and copy/paste that output when reporting the issue:

Get-BusPirateDetails -Verbose -Debug

Created by: henrygab

New Pull Request: Include documentation for building BP6 binaries.
henrygab/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

This improves build documentation.

In particular, it shows how a single enlistment can support building both BP5 (rp2040) binaries, and also building BP5XL/BP6 (rp2350) binaries.

Note: explicitly fetching the SDK for BP6 builds appears necessary, as rp2350 uses a different (and incompatible) SDK version, which is currently too unstable for rp2040.

Created by: henrygab

New Pull Request: Better build directory naming
henrygab/BusPirate5-firmwareDangerousPrototypes/BusPirate5-firmware

Fix some formatting issues with prior update to ./readme.md.

Documentation suggests the use of more descriptive build directory names:

  • ./build_rp2040
  • ./build_rp2350

Build server documentation also now uses the more descriptive build directory names.

Note: Github actions currently continue to simply build in ./build, as they build only one architecture per task, and thus do not have cross-architecture pollution concerns.

Created by: henrygab