Add routines for MMC READ SUBCHANNEL and fix Display of parse errors#13
Add routines for MMC READ SUBCHANNEL and fix Display of parse errors#13skr4n wants to merge 6 commits into
READ SUBCHANNEL and fix Display of parse errors#13Conversation
The display implementation of `ContextError` returns an empty string otherwise.
|
|
||
| return Ok(data); | ||
|
|
||
| const OPCODE: u8 = 0x42; |
There was a problem hiding this comment.
The magic constant 0x42 does not appear in the libcdio C code. Instead, we have in libcdio/cdio/mmc.h:
CDIO_MMC_GPCMD_READ_SUBCHANNEL = 0x42, /**< Read Sub-Channel data. (10 bytes). */Can we remove magic numbers like this?
| let mut cdb = Cdb::default(); | ||
| cdb[0] = OPCODE; | ||
| if let AddressFormat::Time = address_format { | ||
| cdb[1] = 1 << 1; |
There was a problem hiding this comment.
Another magic constant << 1
Consider something like this instead:
const TIME_BIT_SHIFT: usize = 1; // Some place where constants equivalent constants are defined.
...
cdb[1] = 1 << TIME_BIT_SHIFT;
| cdb[1] = 1 << 1; | ||
| } | ||
| if let Some(param) = param { | ||
| cdb[2] = 1 << 6; // subq |
There was a problem hiding this comment.
Consdier instead:
const SUBQ_BIT_SHIFT: usize = 6; // Some place where constants go...
...
cdb[2] = 1 << SUBQ_BIT_SHIFT;
| format_code, | ||
| take(3_usize), // reserved | ||
| mcval, | ||
| take(13_usize), // mcn |
There was a problem hiding this comment.
Consider something like:
const MCN_FIELD_BITS: usize = 13; // Some place where common constants are defined
...
const MCN_FIELD_BITS: usize = 13;Also same thing for // reserved above.
|
When I look at C libcdio's mmc.h header I see lots of information on where to find spec information. For example: https://github.com/libcdio/libcdio/blob/master/include/cdio/mmc.h#L19-L34 and sometimes even down to a section number like https://github.com/libcdio/libcdio/blob/master/include/cdio/mmc.h#L166 . Where/how is this kind of information reflected in this codebase? |
2 participants
Implement all methods that utilize
READ SUBCHANNELcommand.Also, fix parse errors not producing a string in their
Displayimplementations due to a disabled feature flag.