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

[driver] RTC MCP7941X #748

Merged
merged 2 commits into from
Dec 20, 2021
Merged

[driver] RTC MCP7941X #748

merged 2 commits into from
Dec 20, 2021

Conversation

odinthenerd
Copy link
Contributor

somewhere in the initialization I messed something up, I think I need a second pair of eyes

@rleh
Copy link
Member

rleh commented Oct 14, 2021

CI is failing because you have to update the documentation by running the script

$ python3 tools/scripts/synchronize_docs.py

and remove trailing whitespaces.

@odinthenerd
Copy link
Contributor Author

jo #fail, thanks

@rleh
Copy link
Member

rleh commented Oct 14, 2021

Please also include an example for one of the existing development boards.

@rleh rleh self-requested a review October 14, 2021 13:11
@odinthenerd
Copy link
Contributor Author

ok I'll make a black pill board example

src/modm/driver/rtc/mcp7941_impl.hpp Outdated Show resolved Hide resolved
src/modm/driver/rtc/mcp7941.hpp Outdated Show resolved Hide resolved
src/modm/driver/rtc/mcp7941_impl.hpp Outdated Show resolved Hide resolved
src/modm/driver/rtc/mcp7941_impl.hpp Outdated Show resolved Hide resolved
src/modm/driver/rtc/mcp7941.hpp Outdated Show resolved Hide resolved
src/modm/driver/rtc/mcp7941.lb Outdated Show resolved Hide resolved
src/modm/driver/rtc/mcp7941_impl.hpp Outdated Show resolved Hide resolved
src/modm/driver/rtc/mcp7941_impl.hpp Outdated Show resolved Hide resolved
@rleh rleh changed the title initially added mcp7941 using protothreads [driver] RTC MCP7941X Dec 19, 2021
@rleh
Copy link
Member

rleh commented Dec 19, 2021

Works on real hardware ✅

Copy link
Member

@chris-durand chris-durand left a comment

Choose a reason for hiding this comment

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

Thanks!

src/modm/driver/rtc/mcp7941x.hpp Outdated Show resolved Hide resolved
src/modm/driver/rtc/mcp7941x.hpp Outdated Show resolved Hide resolved
src/modm/driver/rtc/mcp7941x.hpp Outdated Show resolved Hide resolved
src/modm/driver/rtc/mcp7941x_impl.hpp Outdated Show resolved Hide resolved
src/modm/driver/rtc/mcp7941x.hpp Outdated Show resolved Hide resolved
@salkinium salkinium merged commit 5a9ad25 into modm-io:develop Dec 20, 2021
@salkinium salkinium added this to the 2021q4 milestone Dec 31, 2021
@rleh rleh mentioned this pull request Jan 1, 2022
15 tasks
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
* chip datasheet available at http://ww1.microchip.com/downloads/en/DeviceDoc/22266A.pdf
Copy link
Member

Choose a reason for hiding this comment

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

Ups.

Copy link
Member

Choose a reason for hiding this comment

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

@rleh What is the issue? Just the formatting or anything else I am to blind to see right now?

Copy link
Member

@salkinium salkinium Jan 1, 2022

Choose a reason for hiding this comment

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

The datasheet link should go into the module.lb instead of the license header. This particular formatting makes it look like it's part of the license as well.

Copy link
Member

Choose a reason for hiding this comment

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

Unrelated, the struct mcp7941x should also have a /// @ingroup modm_driver_mcp7941x on top of it, otherwise doxypress will not group it correctly (sadly).

Copy link
Member

Choose a reason for hiding this comment

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

Line 10 does not belong in the license header. I probably copy-pasted it in there by mistake.

Copy link
Member

Choose a reason for hiding this comment

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

Of course, I was blind ... Will fix it as well.

@salkinium
Copy link
Member

Running doxypress on the example causes a segfault too:

Initialization
Parse input files
Error: Possible recursive class relation while inside modm::mcp7941x and looking for derived class I2cDevice< I2cMaster, 2 >

**  Generate Documentation Output
Error: Possible recursive class relation while inside modm::mcp7941x and looking for base class modm::mcp7941x::DateTime
Error: Possible recursive class relation while inside I2cDevice< I2cMaster, 2 > and looking for base class modm::mcp7941x::DateTime
Error: Possible recursive class relation while inside NestedResumable< 10+1 > and looking for base class modm::mcp7941x::DateTime
Warning: class modm::mcp7941x seem to have a recursive inheritance relation!

@salkinium
Copy link
Member

recursive inheritance relation?

PC LOAD LETTER

I tried a couple of things, nothing fixes this?

@chris-durand
Copy link
Member

This is the line in doxypress where it triggers the message: https://github.com/copperspice/doxypress/blob/1e286c033d4ab8499e42de9016e4f196f66f5f0f/src/classdef.cpp#L2535

It's a doxypress bug. Change the class to

template < class I2cMaster >
class Mcp7941x :  public mcp7941x,

and it appears to work for me. @salkinium Could you try if it works for you? I'll make a PR to fix it if it works.

@salkinium
Copy link
Member

Yes, that works for me both in the example and in the docs generator!

@rleh
Copy link
Member

rleh commented Jan 1, 2022

I can confirm the your fix solves the issue!

template < class I2cMaster >
-class Mcp7941x :       public modm::mcp7941x,
+class Mcp7941x :       public mcp7941x,
					public modm::I2cDevice<I2cMaster, 2>

@chris-durand
Copy link
Member

Ok, I'll make a PR now and later try to fix the bug in doxypress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants