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

core: typedef'ed uint32_t to layer_state_t #526

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alex-ong
Copy link
Contributor

This allows easy further modifications in future such as changing typedef based on number of layers.

@alex-ong
Copy link
Contributor Author

Only part that may not be compatible when changing definition of layer_state_t is:

uint8_t shift = action.layer_bitop.part*4;
layer_state_t bits = ((layer_state_t)action.layer_bitop.bits)<<shift;
layer_state_t mask = (action.layer_bitop.xbit) ? ~(((layer_state_t)0xf)<<shift) : 0;
switch (action.layer_bitop.op) {
    case OP_BIT_AND: default_layer_and(bits | mask); break;
    case OP_BIT_OR:  default_layer_or(bits | mask);  break;
    case OP_BIT_XOR: default_layer_xor(bits | mask); break;
    case OP_BIT_SET: default_layer_and(mask); default_layer_or(bits); break;
}

since it relies on it being 32bits and references in 8 chunks of 4.

Possibly document what action.layer_bitop does, seeing as there are 6 parts to it?
bits(size=4) = the four bits.
xbit(size=1) = if enabled, mask = creates a mask of all 1's apart from the selected part
part(size=3, Values from 0->7) which bits to modify (first four bits, second set of four bits etc)
on(size=2) - not sure what this does yet
op(size=2) - operator - executes the given operator with (bits | mask)
kind(size=4) - action_kind_id

@tmk
Copy link
Owner

tmk commented Jan 24, 2018

THanks.
I'll look into and test that part later.

Yea, that code shifts 4-bit nibble('bits') into plate that 'part' indicates.

    part            7    6    5    4    3    2    1    0                                           
    layer_state     0000 0000 0000 0000 0000 0000 0000 0000                                        
                    msb                                 lsb

shift(part*4) can be 28, 24, 20, 16, 12, 8, 4 or 0 when layer_sate is 32bit; 12, 8, 4 or 0 when 16bit and 4 or 0 when 8bit. We will have to check or truncate shift value depending on size of layer_state to prevent from over shifting like:

#if NUM_LAYERS > 16   // layer_state is unit32_t
    uint8_t shift = action.layer_bitop.part*4;
#elif NUM_LAYERS > 8   // unit16_t
    uint8_t shift = (action.layer_bitop.part & 3)*4;
#else  // uint8_t
    uint8_t shift = (action.layer_bitop.part & 1)*4;
#endif

@alex-ong
Copy link
Contributor Author

alex-ong commented Jan 24, 2018

Yup. I'll do changing NUM_LAYERS as a different request since this request stands fine on its own.
Since this pull request doesn't involve any functionality changes (since type-defs are all still uint32_t) it should be safe to merge.

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.

2 participants