uv5rmini: fix BLE upload with segment-aware offsets and pre-encryptio…#1463
uv5rmini: fix BLE upload with segment-aware offsets and pre-encryptio…#1463Marcotic wants to merge 1 commit intokk7ds:masterfrom
Conversation
c01efb2 to
bd23404
Compare
|
I tried your UV-5R Mini BLE upload fix and it works very well. The only thing you may want to change is the limit for BLE ports to >= 9 to allow COM9 to used as the BLE port as it is a common default port created by com0com installs and it is what most users have configured. Change: Great work. And BTW this address one of the Open issues: https://chirpmyradio.com/issues/12251 If you rename your PR to include "Fixes #12251" in the title the issue will automatically get closed when your PR is merged. |
No it won't. You need to put that magic string in the commit message of the actual commit. The CHIRP bug tracker knows nothing about PRs, only commits. |
kk7ds
left a comment
There was a problem hiding this comment.
Thanks for working on this, I'm sure it will please a lot of users. We need to figure out a way to unify this with the existing code instead of just having a big switch and a different (but almost identical) upload path. I also don't want the radio itself inspecting the serial for the port name. The reason you have to do all the getattr() and hasattr() stuff is an indication that there's a layering violation going on here :)
Thanks!
| return bfc._rawrecv(radio, response_len) | ||
|
|
||
|
|
||
| def _is_ble_port(radio): |
There was a problem hiding this comment.
This needs to be in the common infrastructure somewhere and not in the driver itself. If we need to communicate this down then let's do it, but I don't want drivers to start looking at port names directly.
|
|
||
|
|
||
| def _do_ident_ble(radio, timeout=5.0): | ||
| """BLE identification with relaxed timeout""" |
There was a problem hiding this comment.
The timeout for the radio should be raised and then we don't need to have a different routine here. Unless we're reading the incorrect amount of expected data, the timeout can/should be large. Many drivers in CHIRP try to operate with a very low timeout for no reason.
What's the actual number this needs to be? Surely 5s is way more conservative than it needs to be.. Is 1s sufficient?
| return data | ||
|
|
||
|
|
||
| def _upload_ble(radio): |
There was a problem hiding this comment.
Will this work to upload over serial as well? I would think it does, but it's hard to suss out the exact changes you're making here because the code is duplicated but not the same. If it does work for serial, we should just replace the existing upload routine with this one (or the logic from this one). If it doesn't, I'd like to figure out a way to not just have a fully duplicated upload routine but merge the logic and sequencing changes into the main routine.
I understand this uses larger blocks, but what do you mean by "segment-aware offsets"?
|
BTW, I should say, I'm more than happy to help do the actual work of refactoring this into a unified thing, I just need help understanding the exact changes that are needed between your implementation and the existing one. I can do that by inspection, but it'd be easier if you helped explain... |
I did this cause i was testin with bridge COM10--COM11, changed to COM9--BLE and working good. |
HI!
I've now unified both routines into a single about"segment-aware offsets":
The radio memory is divided into segments (defined by # Calculate actual bytes to read from this segment
bytes_remaining_in_segment = (MEM_START + MEM_SIZE) - addr
bytes_to_read = min(block_size, bytes_remaining_in_segment)
data = radio_mem[data_addr:data_addr + bytes_to_read]This ensures that when we're near the end of a memory segment, we only read the remaining bytes in that segment rather than attempting to read a full 128-byte block that would overflow into the next segment (or beyond). provide any insight? I changed tiemout to 1s at 0.25 it crashed. Also commited wit the suggested text. Hope it's all good and trying to do my best. Apologies if I’m causing confusion. |
Okay that seems to be in conflict with my understanding of what you're changing here. What I need to know is.. why is the current thing not working? If we can read a partial block at the segment boundary, I would think we could just read the smaller blocks all the way through. What happens currently when you try to use the existing USB code over BLE? Timeout short block read, what? I'm worried that we don't really understand what's going on here and just thinking "don't know why but reading larger blocks helps".
Crashed how? Raising the timeout from 0.25 to 1.0 should not make any difference at all, unless we're reading a larger size than is being sent. We should never be doing that, and always be reading the exact amount of data being sent (except for error conditions of course). |
|
I am trying my best to clarify what is going on from my absolute lack of knowledge. Modifications made to enable BLE file upload on a Baofeng 5R Mini Initial symptom: ACK is never received and the process ends in a timeout The failure occurred during the upload when attempting to read the ACK from the radio inside the upload routine. The system tried to receive a single acknowledgment byte (ACK), but it never arrived. The error eventually surfaced as RadioNoContactLikelyK1. The real cause was identified as a read operation from the radio returning an empty value, which means the radio did not respond and the ACK byte (0x06) was never received. Block size mismatch between normal mode and BLE It was discovered that the radio’s BLE firmware expects blocks of 128 bytes, while the original code was sending blocks of 64 bytes. As a result, the BLE firmware did not recognize the transmitted commands, did not return an ACK, and the process timed out, ending in a no-contact error. The required change was to adapt the BLE upload process to operate with 128-byte blocks. Issue when switching to 128-byte blocks: frame construction error When switching to 128-byte blocks, the real underlying problem appeared. The field that represents the block length was packed using a signed byte, whose maximum valid value is 127. The value 128 (0x80) is outside that range, causing the frame to be built incorrectly. The fix was to use an unsigned byte for the length field, allowing values from 0 to 255 and correctly supporting the value 128. With this change, the BLE frame becomes valid and the radio can properly recognize the command and respond with an ACK. Issue at the end of blocks: missing padding with 0xFF With 128-byte blocks, another critical issue appeared. When the data read was smaller than the expected block size, incomplete blocks were being sent. The BLE firmware may reject these blocks or fail to respond with an ACK. The fix was to pad incomplete blocks with the value 0xFF until the block always reaches 128 bytes. The final upload flow is as follows: up to 128 bytes are read, if fewer bytes are read the block is padded with 0xFF, encryption is applied if needed, and a full 128-byte block is always transmitted. This ensures the BLE firmware always receives the exact block size it expects and responds consistently with an ACK. Failure when closing after upload completion: incorrect pointer advancement After the upload completed, a failure occurred during the closing phase due to incorrect calculation of the read pointer advancement. The code advanced the pointer as if a full block had always been read, but in the final block of a segment fewer bytes may be read. The fix was to recalculate on each iteration how many bytes actually remain to be read and to advance the pointer only by the number of bytes that were truly read, rather than always advancing by the full block size. This guarantees that when a segment ends, the pointer reaches the exact expected boundary and the next segment begins at the correct offset, preventing misalignment or closing errors. I modified the file baofeng_uv17Pro.py with the minimum set of changes required to make BLE upload work on the Baofeng 5R Mini. However, this approach breaks the original configuration and behavior. In hindsight, the solution that automatically detected and handled BLE was likely the better approach. It is also worth noting that the timeout-related changes were ultimately unnecessary and were the result of multiple intermediate modifications made during debugging rather than a real requirement of the BLE upload process. With this i hope someone can upoload a better code that resolves the ble problem on these radios. |
|
I missed this one: MEM_STARTS = [0x0000, 0x9000, 0xA000, 0xD000] but for uv5r-Mini MEM_STARTS = [0x0000, 0x9000, 0xA000] Explanation: Problem: The Baofeng UV-5R Mini has only three memory segments, not four. Segment 0: 0x0000–0x8040 (32,832 bytes) Correct total: 0x8240 (33,344 bytes) The 0xD000 segment does not exist on the Mini model and was causing incorrect total memory calculations cuasing a factory reset. |
BLE Upload Fix for Baofeng UV-5R Mini
Description
This PR fixes a critical memory alignment bug in BLE uploads for Baofeng UV-5R Mini radios that caused data corruption and factory reset after upload.
Root Cause
The UV-5R Mini has non-linear memory with gaps between segments:
The original BLE upload logic incremented
data_addrlinearly by 128 bytes (BLE_BLOCK_SIZE) per block, regardless of segment boundaries. When transitioning between non-contiguous segments, this caused the address pointer to overflow memory map boundaries, resulting in reads of uninitialized 0xFF bytes.Additionally, padding was applied AFTER encryption, which meant partial blocks (e.g., 64 bytes) were encrypted first, then padded to 128 bytes—creating a mismatch with the radio's expected format.
Changes
BLE Port Detection (
_is_ble_port()): Detects ble-serial ports and com0com bridges (heuristic: COM10+, "/tmp/ttyble", "/dev/ttyble", or "ble" in port name).BLE Identification (
_do_ident_ble()): Identification sequence with relaxed 5-second timeout for BLE connections.BLE Upload (
_upload_ble()):bytes_remaining_in_segmentfrom segment boundariesdata_addrby actual bytes read, not block sizeUV-5R Mini Auto-Switch (
sync_out()override):_upload_ble()or_upload()accordinglyBLE_BLOCK_SIZE: Added 0x80 (128 bytes) to UV-5R Mini configuration.
Testing
Impact
Notes for Reviewers
_upload()function