Skip to content

add LOLIN_S2_MINI and LOLIN_S2_PICO boards support. #7660

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

Closed
wants to merge 2 commits into from

Conversation

wemos
Copy link
Contributor

@wemos wemos commented Aug 14, 2021

Board information: https://www.wemos.cc/en/latest/s2/s2_mini.html

Signed-off-by: WEMOS support@wemos.cc

BUTTON = const(0)


class led:
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to make this class name lowercase? Usually classes are upper camel case, which in this situation would be LED.

Also, this is a very simple class and machine.Pin already provides the on/off methods. In my opinion it would be simpler to pre-create the LED object like this:

led = Pin(LED, Pin.OUT, value=0)

Then the user can just do s2mini.led.on().

self._led.value(0)


class button:
Copy link
Member

Choose a reason for hiding this comment

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

As with the LED class I think it's much simpler to just do:

button = Pin(BUTTON, Pin.IN, Pin.PULL_UP)

and not have this class at all.

BUTTON = const(0)


class led:
Copy link
Member

Choose a reason for hiding this comment

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

same as for the other board, simpler to use machine.Signal:

led = Signal(LED, Pin.OUT, value=0, invert=True)

self._led.value(1)


class button:
Copy link
Member

Choose a reason for hiding this comment

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

same as for the other board, use button = Pin(BUTTON, Pin.IN, Pin.PULL_UP)

self._btn.irq(trigger=Pin.IRQ_FALLING, handler=cb)


class oled(ssd1306.SSD1306_I2C):
Copy link
Member

Choose a reason for hiding this comment

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

Please use OLED upper camel case.

def __init__(self):
self.reset()

# super().__init__(128,32,I2C(0))
Copy link
Member

Choose a reason for hiding this comment

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

This line should probably be uncommented (init should usually only be called from within init).

time.sleep_ms(10)
Pin(OLED_RST, Pin.OUT).value(1)

super().__init__(128, 32, I2C(0))
Copy link
Member

Choose a reason for hiding this comment

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

This line should probably be removed.

self.text("OLED 128x32", 40, 24, 1)
self.show()

def display_wifi(self):
Copy link
Member

Choose a reason for hiding this comment

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

This function looks useful to debug WiFi but is it good to include it here? I think it would be better as a separate helper module that can be installed from a .py file to the filesystem.

@wemos wemos closed this Aug 16, 2021
@wemos wemos deleted the Branch_5733c49 branch August 16, 2021 06:59
tannewt pushed a commit to tannewt/circuitpython that referenced this pull request Feb 28, 2023
Merge latest 8.0.x changes to main; update frozen libraries
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